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

Add severity and message to logging payload to improve stackdriver log viewing #6761

Closed
diox opened this issue Jun 26, 2019 · 12 comments · Fixed by mozilla/addons-server#12607
Assignees
Labels
qa:not_needed repository:addons-server Issue relating to addons-server
Milestone

Comments

@diox
Copy link
Member

diox commented Jun 26, 2019

Stackdriver currently displays the whole json payload for each line of log because we don't set a message property. Similarly, because we don't set severity all messages are considered to be of the same severity, preventing us to filter by log level.

If we were to add those fields it would increase payload size (since they'd be duplicated) but we'd get better logging viewing experience.

@diox diox self-assigned this Jun 26, 2019
@diox
Copy link
Member Author

diox commented Jun 26, 2019

Note: ideally, all this would be part of some new spec, maybe mozLog 3.0, and handled in https://github.com/mozilla-services/python-dockerflow/blob/master/src/dockerflow/logging.py. In the meantime though I'd like to get nice logs in stackdriver so we'll probably have to implement it ourselves.

@bqbn
Copy link
Contributor

bqbn commented Jun 27, 2019

I think the downside of doing this is probably increased payload, networking traffic and cost. Cost wise, we currently emit about 5 GB application logs per day. Doubling that should not be a huge deal.

Also maybe this can be made so that it can be switched on/off by an environment variable, so that if/when we stop using stackdriver, we can turned it off. The scenario is less likely to happen near-term, but given that we have changed our logging platform a few times, it's not entirely impossible. :P

@diox diox changed the title Add severy and message to logging payload to improve stackdriver log viewing Add severity and message to logging payload to improve stackdriver log viewing Aug 12, 2019
@diox
Copy link
Member Author

diox commented Sep 26, 2019

Once a new release of python-dockerflow includes the changes I made in mozilla-services/python-dockerflow#37 we'll be able to customize the payload more easily.

@diox
Copy link
Member Author

diox commented Oct 7, 2019

we merged mozilla/addons-server#12439 which means we're now using a python-dockerflow version including those changes.

@diox
Copy link
Member Author

diox commented Oct 7, 2019

@bqbn I have a patch locally that works. Do you have an opinion regarding duplicating the message vs removing Fields['msg'] from the payload entirely and instead emit it as message ? I can do either, behind an env variable as you suggested. The latter would kinda be breaking mozLog 2.0 spec though.

@willdurand
Copy link
Member

FWIW, I personally use custom fields in stackdriver: https://cloud.google.com/logging/docs/view/overview#custom-fields

I have this one configured all the time:

jsonPayload.Fields.msg

@diox
Copy link
Member Author

diox commented Oct 7, 2019

Would it be a problem to switch to message instead ? I guess we'd try it on dev in any case.

@bqbn
Copy link
Contributor

bqbn commented Oct 7, 2019

The latter would kinda be breaking mozLog 2.0 spec though.

If this is true, then I prefer the former for the reason that it doesn't break the spec.

The logging platform and tools built around it are shared across all services cloudops has to manage, and it's assumed that the spec is adhered to. If we have something that breaks the spec, I fear that it'll cause problems in the future, say, when new feature or tools are introduced to the logging platform.

@diox diox added this to the 2019.10.17 milestone Oct 8, 2019
@diox
Copy link
Member Author

diox commented Oct 14, 2019

FWIW, I personally use custom fields in stackdriver: https://cloud.google.com/logging/docs/view/overview#custom-fields

I do too but the problem with custom fields is that you can't remove the big json blob column when you use them, so the view is still a little crowded, with lots of extra garbage being displayed. Worse, if the message is a bit too long, it gets truncated in favor of the json blob column.

@diox
Copy link
Member Author

diox commented Oct 14, 2019

@bqbn I might just set severity for now and ignore message, cause the duplication of the text bothers me. Do you know if anything in cloudops logging pipeline/services would have issues if I add a severity set in addition to the Severity field that is already set? JSON is case-sensitive, and Stackdriver should be as well.

A typical record would then contain:

{
    //...
    "Severity": 7,
    "severity": 100
}

(Note the different values, since MozLog's Severity (uppercase s) are syslog levels, and Stackdriver's severity (lowercase s) are different.

@bqbn
Copy link
Contributor

bqbn commented Oct 14, 2019

Quote from Slack,

@diox i don't remember the context of the question, but right off the bat, cloudops doesn't manipulate either field, Severity or severity, if there's going to be an issue, it's going to be either stackdriver, the flentd agent or the mozLog client lib.

@diox
Copy link
Member Author

diox commented Oct 15, 2019

Verified myself, it's working, we now have severity display/filtering in stackdriver for addons-server dev:

severity

@KevinMind KevinMind transferred this issue from mozilla/addons-server May 4, 2024
@KevinMind KevinMind added repository:addons-server Issue relating to addons-server migration:2024 labels May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qa:not_needed repository:addons-server Issue relating to addons-server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants