Closed Bug 1066323 Opened 6 years ago Closed 1 year ago

[mozlog] Mozlog shouldn't propagate unremarkable defaults to the raw log

Categories

(Testing :: Mozbase, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(firefox69 fixed)

RESOLVED FIXED
mozilla69
Tracking Status
firefox69 --- fixed

People

(Reporter: chmanchester, Assigned: ifeanyichukwunwabuokei, Mentored)

References

(Blocks 1 open bug)

Details

(Keywords: good-first-bug)

Attachments

(1 file)

I noticed some of the raw JSON logs for mochitest have a lot of "exc_info": false, and some raw JSON logs to marionette have empty "extra" dicts. These don't show up always and don't do real harm, but we should probably prevent unremarkable defaults from ending up in the raw log.
Summary: Mozlog shouldn't propagate unremarkable defaults to the raw log → [mozlog] Mozlog shouldn't propagate unremarkable defaults to the raw log
Mentor: james
Keywords: good-first-bug

I am an Outreachy applicant and would like to work on this.

Assignee: nobody → ifeanyichukwunwabuokei

Hi James. I understand most of these good first bugs already have someone assigned - am I able to work on this one as well please?
Alternatively, if there's another bug that you think might be a good glimpse into the code (whilst not first bug material), I'd love to take a look! Eg. https://bugzilla.mozilla.org/show_bug.cgi?id=1030228, https://bugzilla.mozilla.org/show_bug.cgi?id=1021926
Thanks!

Nikki: Yeah, we have a shortage of good first bugs, but I'm happy for you either to try this one or look at bug 1021926. The other one you mentioned may not be relevant any more; I'm not sure.

@James please help me out. I am a bit confused.

Apologies for the lack of good instructions in the issue :)

So the log_raw method is here [1]. It takes a dictionary that looks like the internal log format but uses it as the input to the logging pipeline (this is helpful for stitching together code written in different languages). The issue is that sometimes log entries have default values that should be elided from the log. These are marked as optional=True in the API e.g. [2]. So to use that example, if I make a call like

logger.log_raw({"action": "test_start", "test":"my-example-test", "path": None})

and I look at the message that gets passed to the handlers, it will explicitly have "path": None in a field, whereas if I run something like

logger.test_start(test="my-example-test", path=None)

there won't be any path in the output. So the intent of the bug is that where the field is optional and has a default and the value matches the default we just don't add it to the output. To do this, I suggest looking at the log_action class [3] which has the list of types accepted by each method and their properties. You could for example add an extra method that could be used in conjunction with convert_known and returns a list of provided proeprties that match their default value. Then these could be filtered out when we re-add missing properties in the log_raw method.

[1] https://searchfox.org/mozilla-central/source/testing/mozbase/mozlog/mozlog/structuredlog.py#226-247
[2] https://searchfox.org/mozilla-central/source/testing/mozbase/mozlog/mozlog/structuredlog.py#332
[3] https://searchfox.org/mozilla-central/source/testing/mozbase/mozlog/mozlog/logtypes.py#17

Hi! @jgraham
Can I also work on this if there is no patch yet?
Thanks and Regards,
Shruthi

Yeah, unfortunately there aren't enough suitable issues to go around, so I'm happy for multiple people to try looking at the same issue.

Hi @jgraham, I have a few doubts:

  1. When I try to run test_Structured.py, I get an error saying No Module Named Mozunit. How can I resolve it? I would like to run and see how it works before making the change. Please help.

  2. I tried to run the functions involved in log_raw separately but was not able to understand the role of convert which is called by convert known. Can you please tell me?

  3. Based on the understanding from your explanation, I should add a method in logtypes.py to check if the message contains properties with default values and remove them and stop them from being re-added right?

Thank you in advance.

Flags: needinfo?(james)

To run the tests, you need to run ./mach python-test from the root of your checkout. You should be able to pass in the path to the specific test file you want.

TO understand the convert method, note that we statically define the type allowed for each argument as a property of the log_action class. These types are represented by the DataType subclasses defined in the logtypes.py file are are stored in self.args which is a mapping of argument name to type. Each DataType subclass defines a __call__ method that does type validation and conversion for the specified type. So the essence of this function is just to call the __call__ method of the DataType subclass corresponding to each argument passed to the logger and return a dictionary of {arg_name: DataType_instance.__call__(arg_value)}. Where things get complex is that python calling conventions are complex, so we need to accept a mix of keyword and positional arguments (one can do things like def f(a,b,c=3) and call as f(1, c=3, b=2)).

I think your understanding of the way forward is correct. I hope that explaination helps, let me know if you are still confused or have any other questions.

Flags: needinfo?(james)

Currently, some of the raw JSON logs for mochitest and marionette, et al, include
empty dictionaries, None values and other unremarkable values that are marked
as optional. This fix aims to remove these unnecessary items from being
passed to the raw log.

A method has been added to the log_actions class which removes defaults if they
are marked as optional and the value is included in the default list. This is
called on the kwargs returned by the convert_known method, before being
propagated to the log_raw method for StructuredLogger.

Attachment #9053940 - Attachment description: Bug 1066323 - Mozlog shouldn't propagate unremarkable defaults to the raw log → Bug 1066323 - [mozlog] Mozlog shouldn't propagate unremarkable defaults to the raw log
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/86bc425ccdbd
[mozlog] Mozlog shouldn't propagate unremarkable defaults to the raw log r=jgraham
Flags: needinfo?(ifeanyichukwunwabuokei) → needinfo?(nsharpley)
Pushed by james@hoppipolla.co.uk:
https://hg.mozilla.org/integration/autoland/rev/96aeb5e59c66
[mozlog] Mozlog shouldn't propagate unremarkable defaults to the raw log r=jgraham
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla69
Flags: needinfo?(nsharpley)
You need to log in before you can comment on or make changes to this bug.