Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial attempt at dispatch-553 #109

Closed
wants to merge 2 commits into from
Closed

Conversation

mgoulish
Copy link
Contributor

@mgoulish mgoulish commented Nov 3, 2016

Ted -- I don't know yet how to test the json stuff so I am flying blind there, and I do not yet have the entity-refresh-cache thing. But I wanted to send you what I do have now... just because.

@@ -841,6 +841,46 @@
"type": "string",
"description": "Where to send log messages. Can be 'stderr', 'stdout', 'syslog' or a file name.",
"update": true
},
"default": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These attributes should have names that are more descriptive and should suggest that they are counters.

"default": {
"type": "integer",
"description": "how many entries in the severity-level histogram at the DEFAULT level",
"update": "true"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEFAULT is not a level that is used in raising logs. It is only used in setting the enabled log threshold. In other words, it should not be counted as it will always be zero.

@@ -149,13 +149,16 @@ static log_sink_t* log_sink_lh(const char* name) {
}


typedef enum {DEFAULT, NONE, TRACE, DEBUG, INFO, NOTICE, WARNING, ERROR, CRITICAL, N_LEVELS} level_index_t;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DEFAULT and NONE are not used in raising logs.

We can always decide not to look at it later,
based on its used/unused status.
-----------------------------------------------*/
source->severity_histogram [ level ] ++;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have a type problem here. qd_log_level_t can't be used as an index into an array since it is a bitmask.

"trace": {
"type": "integer",
"description": "how many entries in the severity-level histogram at the TRACE level",
"update": "true"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't use update:true here. This would allow a remote user to update the value of the counter. This is a read-only quantity.

@mgoulish mgoulish closed this Nov 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants