-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Adds minimal traceparent header support to Elasticsearch backport(#74210) #75331
Conversation
This adds just enough support for the traceparent header to be useful in es8. Since Elasticsearch already logs in ECS format extending it with support for transaction.id and trace.id is a quick win. This allows us to surface server/deprecation slow logs from an instrumented application using the Trace Logs feature. Parsing `traceparent` in http layer and populating tasks with `trace_id` which is preserved in thread context.
Pinging @elastic/es-core-infra (Team:Core/Infra) |
@@ -63,7 +63,7 @@ appender.deprecation_rolling.name = deprecation_rolling | |||
appender.deprecation_rolling.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_deprecation.json | |||
appender.deprecation_rolling.layout.type = ESJsonLayout | |||
appender.deprecation_rolling.layout.type_name = deprecation.elasticsearch | |||
appender.deprecation_rolling.layout.esmessagefields=x-opaque-id | |||
appender.deprecation_rolling.layout.esmessagefields=x-opaque-id,key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it was already being set in https://github.com/elastic/elasticsearch/pull/75331/files#diff-585bbd41ac3ed6c6aa4811cecbd0d3e776af7c594e3ae8558fa5434fbfa3bcd6R38
so I thought why not have it in logs too
@@ -310,6 +310,14 @@ public static String getXOpaqueId() { | |||
.orElse(""); | |||
} | |||
|
|||
public static String getTraceId() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this called anywhere? TraceIdConverter
has its own implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good find. I have removed that method.
elastic#75331 missed adding trace.id to indexed deprecation logs (they use ECSJsonLayout instead of ESJsonLayout).
#75331 missed adding trace.id to indexed deprecation logs (these use ECSJsonLayout instead of ESJsonLayout).
This adds just enough support for the traceparent header to be useful in es7
Contrary to es8 in 7.x branch the logs are in custom JSON format - not in ECS.
trace.id is added to deprecation, slow logs and server JSON logs
backport #74210