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

Unformatted trace logging for step telemetry in ExecutionContext #3581

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

carlosdagos
Copy link

Don't apply JSON formatting during the trace logging for step telemetry. By default StringUtil.ConvertToJson uses Formatting.Indented which causes multi-line logs for this particular json object.

The data contained in this particular log line is useful for operators of self-hosted runners who are looking to use log aggregates to understand their overall usage and performance. Parsing multi-line data in log aggregates is very hard to get right, if not outright impossible depending on how logs are aggregated.

Note to reviewers

Unfortunately I found it hard to add a test for this (and arguably adding a test for a Trace is perhaps not something one may want to do?) given that the Tracing object is private so testing via something like TraceListener looked like it would entail a lot more changes. However the test logs confirm that indeed it now prints the relevant log in a single line rather than multiple

image

Don't apply JSON formatting during the trace logging for step telemetry.
By default `StringUtil.ConvertToJson` uses `Formatting.Indented` which
causes multi-line logs for this particular json object.

The data contained in this particular log line is useful for operators
of self-hosted runners who are looking to use log aggregates to
understand their overall usage and performance. Parsing multi-line data
in log aggregates is very hard to get right, if not outright impossible
depending on how logs are aggregated.
@carlosdagos carlosdagos requested a review from a team as a code owner November 19, 2024 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant