[bugfix] Send test attributes correctly with the Graylog handler #1644
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 6 commits
December 4, 2020 00:38
The log record extras are not formatted at all when they are defined in the `LoggerAdapter`'s extras. They can be raw objects. Our custom formatter will format them, but it will NOT overwrite the log record attributes with their formatted values, so that the record contains all the information. The builtin formatters can still work with that, except that they will use the `str()` method to format objects. The Graylog handler does not use a formatter by design. Instead, it encodes the whole record object into JSON and sends it over to the Graylog server. All we do in this case is to use our own JSON encoder, which can recognize the ReFrame objects.
Codecov Report
@@ Coverage Diff @@
## master #1644 +/- ##
==========================================
+ Coverage 87.54% 87.57% +0.02%
==========================================
Files 44 44
Lines 7288 7289 +1
==========================================
+ Hits 6380 6383 +3
+ Misses 908 906 -2
Continue to review full report at Codecov.
|
added 2 commits
December 5, 2020 21:05
teojgo
requested changes
Dec 7, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The implementation of the dynamic log record based on arbitrary test attributes is revised completely. It does not happen anymore in the formatter, because the formatter is not used by the Graylog handler and this was the reason for #1640. All public test attributes as well as some interesting properties are put in the logger's
extradict. This will be transferred to the log record by Python's logging mechanism. At this point, we do NOT format anything, except thecheck_job_completion_time, in order to be sent correctly to Graylog. The formatting of the record is an exclusive duty of the formatter object, which formats the final log message through itsformatMessage()method. We don't format the record attributes directly, but we use a proxy dictionary, which holds the formatted values, in order to produce the final message from the format string. All attributes, except lists, tuples, sets and strings are formatted using ourjsonext.dumps()function. For compatibility with the current behaviour, lists, tuples and sets are formatted as comma-separated lists, whereas strings are passed as-is (NB: if they were JSON formatted, they would be quoted). This allows users to use the%(check_perf_patterns)sformat specifier in order to get all the performance variables and values at once as a JSON object. If a requested test attribute cannot be found, it will be logged asnull.The Graylog handler sends to the remote end the full log record converted to JSON. Again here, we use our JSON encoding method. There are some limitations to this. If a field is logged as
nullit will not be indexed and it will not show up in the Kibana logs. Also, for some reason that I cannot understand, the same happens for boolean fields. It would be rather complicated to format these fields as strings and transfer them, without messing with the log record, so I chose not to do it, because in principle it's not a handler's responsibility to format a log record.Fixes #1640.