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 trace.id to indexed deprecation logs #81524

Merged

Conversation

pgomulka
Copy link
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
Collaborator

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

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

pgomulka commented Dec 8, 2021

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

@@ -52,7 +52,7 @@ public static String getTraceId() {
public void format(LogEvent event, StringBuilder toAppendTo) {
String traceId = getTraceId();
if (traceId != null) {
toAppendTo.append("\"trace.id\": \"" + traceId + "\"");
toAppendTo.append(traceId);
Copy link
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
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
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
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