Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

feat(ngLog): provides support to debug the application in chosen level #8303

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

feat(ngLog): provides support to debug the application in chosen level #8303

wants to merge 3 commits into from

Conversation

gabrielfgularte
Copy link

Request Type: feature

How to reproduce:

Component(s): jqLite

Impact: large

Complexity: small

This issue is related to:

Detailed Description:

I think we need to set a level to debug the application. So, I've developed a feature that add an extra behaviour to $log. In the $logProvider, I created a method called debugLevel that receives a string as parameter and, based on that parameter, the application change its debug level.

With this feature, setting debugLevel to 'error' will log only error messages, while setting 'log' will debug the application in a "trace like" mode.

  • error: log only error messages;
  • warn: log warn and error messages;
  • info: log info, warn and error messages;
  • log: log log and all other messages (trace like).

I'm not able to write tests for this feature and I think the code can be cleaner, but it helped me a lot. In the two last commits I've added two tests to this feature: one to ensure that if debugLevel is not defined, the application will log as default and one setting the level to 'warn'.

Other Comments:

ngLog can now log in window console based on a debug level, that can be redefined in LogProvider. Defaults to 'log'.

ngLog can now log in window console based on a debug level, that can be
redefined in LogProvider. Defaults to 'debug'.
@mary-poppins
Copy link

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

  • Uses the issue template (#8303)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@jeffbcross jeffbcross self-assigned this Jul 22, 2014
@jeffbcross jeffbcross added this to the Backlog milestone Jul 22, 2014
@jeffbcross jeffbcross removed their assignment Jul 22, 2014
@gabrielfgularte
Copy link
Author

@jeffbcross Please, take a look at this. I just pushed a chore that changes the log hierarchy and added tests. Thanks!

@gabrielfgularte gabrielfgularte changed the title Debug level feat(ngLog): provides support to debug the application in chosen level Jul 28, 2014
@btford btford removed the gh: PR label Aug 20, 2014
@thebigredgeek
Copy link
Contributor

I actually think this approach assumes too much. I'd actually prefer something like this:

#8832

It provides more granular control over logging, and makes less assumptions. I don't think the framework should be making decisions regarding logging tier hierarchies and disabling them en-masse based on predefined strings.

@gabrielfgularte
Copy link
Author

@thebigredgeek I've worked with some server-side frameworks like Yii (php), Django, Flask and Tornado (python). And in all of them, as far as I know, you don't choose or toggle each kind of log, you only choose the Application Debug Level. Obviously, Angular.js does not need to be like such frameworks and it can be different in very features and aspects, but what I'm trying to do, is bring the power we have in those frameworks to Angular.js and use it in the same way, creating a pattern to be followed by developers independent of the framework or language.

Some good links:
http://flask.pocoo.org/docs/0.10/errorhandling/
https://docs.djangoproject.com/en/dev/topics/logging/
http://docs.nodejitsu.com/articles/intermediate/how-to-log
https://www.npmjs.org/package/loglevel
http://www.yiiframework.com/doc/guide/1.1/pt/topics.logging

@thebigredgeek
Copy link
Contributor

I understand that this is somewhat normal, but what I don't understand is why you would want less control over logging? It seems a bit ridiculous to want string-based logger toggling abstraction rather than boolean-based granular control over each type of logger entity. Can you explain what benefit a more abstract approach offers versus just extended the same functionality that is already present? Your approach would also be a breaking change, as there are many people who are already using $logProvider with its current functional signature.

@gabrielfgularte
Copy link
Author

"...why you would want less control over logging?"

This feature will add more control over logging by changing your level of debug in determined environments.

"Can you explain what benefit a more abstract approach offers versus just extended the same functionality that is already present?"

It's not a change, the existing functionality will be intact and can be extended. $log.debug still can be toggled with $logProvider.debugEnabled([true | false]). What I'm trying to do, is provide a way to spread any kind of log at any point of the application and see only the level logs you want to see on a specific environment. I think it's very useful when I want to see all kind of logs in development, but not in production, where I only need or want to see warnings and errors logs. Let me show you an example on how I think it will work:

angular.module('myModule')
.config([
  '$logProvider',
  function($logProvider) {
    // set the level based on your environment or on your needs of debugging
    $logProvider.debugLevel('info');
  }
])
.controller('MyController', [
  '$scope',
  '$log',
  function($scope, $log) {

    $scope.myFunction = function(user) {

      $log.log('entered in myFunction');

      if ($scope.checkIfUserExists(user)) {
        $log.warn('user already exists');
        return;
      }

      if ($scope.saveUser(user)) {

        $log.info('user saved');

      } else {

        $log.error('error on save user');

      }

    };

  }
]);

@thebigredgeek
Copy link
Contributor

Again, I am not sure why we would want to create pre-seeded logging "template" settings like this, rather than doing something like:

angular.module('foo').config([
  '$logProvider',
  function($logProvider){
    $logProvider.debugEnabled(false);
    $logProvider.infoEnabled(false);
    $logProvider.warnEnabled(false);
  });

I think your approach is great, but I also think it is far too opinionated to be built into angular core. If you want to create string-based levels, you can just roll your own provider around this very schema. It's unopinionated and low level, which IMO is how it should be.

TL;DR; - I think we want to keep Angular core pretty unopinonated. I think that something like environment-base log configuration should be deferred to the programmer implementing. Rolling your own provider around an extension of the existing $logProvider api is simple enough.

@gabrielfgularte
Copy link
Author

All I can say, is this is just an easy-add feature that all other frameworks that I've been worked provide to me. If you want to add a "single-level-debug" control feature, you can do it. I think it will not conflict with this feature. But, in my opinion, "debug-level-hierarchy" is more efficient and its pattern can be found in all major frameworks that I've seen.

@nicholasess
Copy link

+1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants