Skip to content

Add trace.id to indexed deprecation logs#81524

Merged
pgomulka merged 2 commits intoelastic:7.16from
pgomulka:7.16_missing_trace_id_in_indexed_logs
Dec 9, 2021
Merged

Add trace.id to indexed deprecation logs#81524
pgomulka merged 2 commits intoelastic:7.16from
pgomulka:7.16_missing_trace_id_in_indexed_logs

Conversation

@pgomulka
Copy link
Copy Markdown
Contributor

@pgomulka pgomulka commented Dec 8, 2021

#75331 missed adding
trace.id to indexed deprecation logs (these use ECSJsonLayout instead of
ESJsonLayout).

  • Have you signed the contributor license agreement?
  • Have you followed the contributor guidelines?
  • If submitting code, have you built your formula locally prior to submission with gradle check?
  • If submitting code, is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed.
  • If submitting code, have you checked that your submission is for an OS and architecture that we support?
  • If you are submitting this code for a class then read our policy for that.

elastic#75331 missed adding
trace.id to indexed deprecation logs (they use ECSJsonLayout instead of
ESJsonLayout).
@pgomulka pgomulka added :Core/Infra/Logging Log management and logging utilities v7.16.1 labels Dec 8, 2021
@pgomulka pgomulka self-assigned this Dec 8, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Dec 8, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

@pgomulka pgomulka added the >bug label Dec 8, 2021
@pgomulka
Copy link
Copy Markdown
Contributor Author

pgomulka commented Dec 8, 2021

@elasticmachine run elasticsearch-ci/packaging-tests-windows-sample

String traceId = getTraceId();
if (traceId != null) {
toAppendTo.append("\"trace.id\": \"" + traceId + "\"");
toAppendTo.append(traceId);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's the reason for this change? Is it so that we can reuse the traceId formatter in EcsJsonLayout.java?

I wonder if this changes behaviour a bit, previously if traceId was null the format would not append anything, but now we might get "trace.id:null". Is that OK?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe notEmpty is smart enough ? :)

Copy link
Copy Markdown
Contributor Author

@pgomulka pgomulka Dec 8, 2021

Choose a reason for hiding this comment

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

yes, notEmpty will check if any of the variables in its "body" are evaluated .
There should be no field name if the value is null
and yes, the reason for this change was to reuse it in ECSJasonLayout

Copy link
Copy Markdown
Contributor

@grcevski grcevski left a comment

Choose a reason for hiding this comment

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

LGTM!

@pgomulka pgomulka merged commit 6bef861 into elastic:7.16 Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Core/Infra/Logging Log management and logging utilities Team:Core/Infra Meta label for core/infra team v7.16.1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants