From 39f00ab0772f18590491dfb5696e18b532b8fcc6 Mon Sep 17 00:00:00 2001 From: Martijn Laarman Date: Mon, 21 Jun 2021 16:27:59 +0200 Subject: [PATCH 1/4] Adds minimal traceparent header support to Elasticsearch (#74210) 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. --- .../common/logging/JsonLoggerTests.java | 20 +++++-- .../elasticsearch/action/ActionModule.java | 5 +- .../common/logging/DeprecatedMessage.java | 3 +- .../common/logging/TraceIdConverter.java | 59 +++++++++++++++++++ .../common/util/concurrent/ThreadContext.java | 19 +++--- .../java/org/elasticsearch/node/Node.java | 2 +- .../elasticsearch/rest/RestController.java | 55 ++++++++++------- .../java/org/elasticsearch/tasks/Task.java | 13 ++++ .../rest/RestControllerTests.java | 33 +++++++++++ 9 files changed, 174 insertions(+), 35 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/common/logging/TraceIdConverter.java diff --git a/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java b/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java index b7a6947c21695..f065fb27a1bd8 100644 --- a/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java +++ b/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java @@ -73,9 +73,12 @@ public void tearDown() throws Exception { super.tearDown(); } - public void testParseFieldEmittingLogs() throws Exception { + + + public void testParseFieldEmittingDeprecatedLogs() throws Exception { withThreadContext(threadContext -> { threadContext.putHeader(Task.X_OPAQUE_ID, "someId"); + threadContext.putHeader(Task.TRACE_ID, "someTraceId"); ParseField deprecatedField = new ParseField("new_name", "deprecated_name"); assertTrue(deprecatedField.match("deprecated_name", LoggingDeprecationHandler.INSTANCE)); @@ -102,7 +105,10 @@ public void testParseFieldEmittingLogs() throws Exception { hasEntry("cluster.name", "elasticsearch"), hasEntry("node.name", "sample-name"), hasEntry("message", "Deprecated field [deprecated_name] used, expected [new_name] instead"), - hasEntry("x-opaque-id", "someId") + hasEntry(DeprecatedMessage.KEY_FIELD_NAME, "deprecated_field_deprecated_name"), + hasEntry(DeprecatedMessage.X_OPAQUE_ID_FIELD_NAME, "someId"), + hasEntry(Task.TRACE_ID, "someTraceId"), + hasEntry("elasticsearch.event.category", "api") ), allOf( hasEntry("type", "deprecation.elasticsearch"), @@ -111,7 +117,10 @@ public void testParseFieldEmittingLogs() throws Exception { hasEntry("cluster.name", "elasticsearch"), hasEntry("node.name", "sample-name"), hasEntry("message", "Deprecated field [deprecated_name2] used, expected [new_name] instead"), - hasEntry("x-opaque-id", "someId") + hasEntry(DeprecatedMessage.KEY_FIELD_NAME, "deprecated_field_deprecated_name2"), + hasEntry(DeprecatedMessage.X_OPAQUE_ID_FIELD_NAME, "someId"), + hasEntry(Task.TRACE_ID, "someTraceId"), + hasEntry("elasticsearch.event.category", "api") ) ) @@ -126,6 +135,7 @@ public void testParseFieldEmittingLogs() throws Exception { public void testDeprecatedMessage() throws Exception { withThreadContext(threadContext -> { threadContext.putHeader(Task.X_OPAQUE_ID, "someId"); + threadContext.putHeader(Task.TRACE_ID, "someTraceId"); final DeprecationLogger testLogger = DeprecationLogger.getLogger("org.elasticsearch.test"); testLogger.deprecate(DeprecationCategory.OTHER, "someKey", "deprecated message1"); @@ -147,7 +157,8 @@ public void testDeprecatedMessage() throws Exception { hasEntry("cluster.name", "elasticsearch"), hasEntry("node.name", "sample-name"), hasEntry("message", "deprecated message1"), - hasEntry("x-opaque-id", "someId") + hasEntry(DeprecatedMessage.KEY_FIELD_NAME, "a key"), + not(hasKey(DeprecatedMessage.X_OPAQUE_ID_FIELD_NAME)) ) ) ); @@ -157,7 +168,6 @@ public void testDeprecatedMessage() throws Exception { }); } - public void testDeprecatedMessageWithoutXOpaqueId() throws IOException { final DeprecationLogger testLogger = DeprecationLogger.getLogger("org.elasticsearch.test"); testLogger.deprecate(DeprecationCategory.OTHER, "a key", "deprecated message1"); diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java index c3ab89245dedb..01f4336da07e4 100644 --- a/server/src/main/java/org/elasticsearch/action/ActionModule.java +++ b/server/src/main/java/org/elasticsearch/action/ActionModule.java @@ -452,7 +452,10 @@ public ActionModule(boolean transportClient, Settings settings, IndexNameExpress destructiveOperations = new DestructiveOperations(settings, clusterSettings); Set headers = Stream.concat( actionPlugins.stream().flatMap(p -> p.getRestHeaders().stream()), - Stream.of(new RestHeaderDefinition(Task.X_OPAQUE_ID, false)) + Stream.of( + new RestHeaderDefinition(Task.X_OPAQUE_ID, false), + new RestHeaderDefinition(Task.TRACE_PARENT, false) + ) ).collect(Collectors.toSet()); UnaryOperator restWrapper = null; for (ActionPlugin plugin : actionPlugins) { diff --git a/server/src/main/java/org/elasticsearch/common/logging/DeprecatedMessage.java b/server/src/main/java/org/elasticsearch/common/logging/DeprecatedMessage.java index ecf5d0678951b..12f5fe9b9a96d 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/DeprecatedMessage.java +++ b/server/src/main/java/org/elasticsearch/common/logging/DeprecatedMessage.java @@ -21,6 +21,7 @@ */ public class DeprecatedMessage extends ESLogMessage { public static final String X_OPAQUE_ID_FIELD_NAME = "x-opaque-id"; + public static final String KEY_FIELD_NAME = "key"; public DeprecatedMessage(DeprecationCategory category, String key, String xOpaqueId, String messagePattern, Object... args) { super(fieldMap(category, key, xOpaqueId), messagePattern, args); @@ -35,7 +36,7 @@ private static Map fieldMap(DeprecationCategory category, String builder.put("category", category.name().toLowerCase(Locale.ROOT)); if (Strings.isNullOrEmpty(key) == false) { - builder.put("key", key); + builder.put(KEY_FIELD_NAME, key); } if (Strings.isNullOrEmpty(xOpaqueId) == false) { builder.put(X_OPAQUE_ID_FIELD_NAME, xOpaqueId); diff --git a/server/src/main/java/org/elasticsearch/common/logging/TraceIdConverter.java b/server/src/main/java/org/elasticsearch/common/logging/TraceIdConverter.java new file mode 100644 index 0000000000000..d57fa4a0ae1bd --- /dev/null +++ b/server/src/main/java/org/elasticsearch/common/logging/TraceIdConverter.java @@ -0,0 +1,59 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +package org.elasticsearch.common.logging; + +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.config.plugins.Plugin; +import org.apache.logging.log4j.core.pattern.ConverterKeys; +import org.apache.logging.log4j.core.pattern.LogEventPatternConverter; +import org.apache.logging.log4j.core.pattern.PatternConverter; +import org.elasticsearch.tasks.Task; + +import java.util.Objects; + +/** + * Pattern converter to format the trace id provided in the traceparent header into JSON fields trace.id. + */ +@Plugin(category = PatternConverter.CATEGORY, name = "TraceIdConverter") +@ConverterKeys({"trace_id"}) +public final class TraceIdConverter extends LogEventPatternConverter { + /** + * Called by log4j2 to initialize this converter. + */ + public static TraceIdConverter newInstance(@SuppressWarnings("unused") final String[] options) { + return new TraceIdConverter(); + } + + public TraceIdConverter() { + super("trace_id", "trace_id"); + } + + public static String getTraceId() { + return HeaderWarning.THREAD_CONTEXT.stream() + .map(t -> t.getHeader(Task.TRACE_ID)) + .filter(Objects::nonNull) + .findFirst() + .orElse(null); + } + + /** + * Formats the trace.id into json fields. + * + * @param event - a log event is ignored in this method as it uses the clusterId value + * from NodeAndClusterIdStateListener to format + */ + @Override + public void format(LogEvent event, StringBuilder toAppendTo) { + String traceId = getTraceId(); + if (traceId != null) { + toAppendTo.append(traceId); + } + } + +} diff --git a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java index 34842ab8fd10e..07bb7ecf2957c 100644 --- a/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java +++ b/server/src/main/java/org/elasticsearch/common/util/concurrent/ThreadContext.java @@ -11,7 +11,6 @@ import org.apache.logging.log4j.Logger; import org.elasticsearch.action.support.ContextPreservingActionListener; import org.elasticsearch.client.OriginSettingClient; -import org.elasticsearch.common.collect.MapBuilder; import org.elasticsearch.core.Tuple; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; @@ -109,15 +108,21 @@ public StoredContext stashContext() { /** * X-Opaque-ID should be preserved in a threadContext in order to propagate this across threads. * This is needed so the DeprecationLogger in another thread can see the value of X-Opaque-ID provided by a user. + * The same is applied to Task.TRACE_ID. * Otherwise when context is stash, it should be empty. */ - if (context.requestHeaders.containsKey(Task.X_OPAQUE_ID)) { - ThreadContextStruct threadContextStruct = - DEFAULT_CONTEXT.putHeaders(MapBuilder.newMapBuilder() - .put(Task.X_OPAQUE_ID, context.requestHeaders.get(Task.X_OPAQUE_ID)) - .immutableMap()); + if (context.requestHeaders.containsKey(Task.X_OPAQUE_ID) || context.requestHeaders.containsKey(Task.TRACE_ID)) { + Map map = new HashMap<>(2, 1); + if (context.requestHeaders.containsKey(Task.X_OPAQUE_ID)) { + map.put(Task.X_OPAQUE_ID, context.requestHeaders.get(Task.X_OPAQUE_ID)); + } + if (context.requestHeaders.containsKey(Task.TRACE_ID)) { + map.put(Task.TRACE_ID, context.requestHeaders.get(Task.TRACE_ID)); + } + ThreadContextStruct threadContextStruct = DEFAULT_CONTEXT.putHeaders(map); threadLocal.set(threadContextStruct); - } else { + } + else { threadLocal.set(DEFAULT_CONTEXT); } return () -> { diff --git a/server/src/main/java/org/elasticsearch/node/Node.java b/server/src/main/java/org/elasticsearch/node/Node.java index c908f3c25270e..6ebc724b38973 100644 --- a/server/src/main/java/org/elasticsearch/node/Node.java +++ b/server/src/main/java/org/elasticsearch/node/Node.java @@ -641,7 +641,7 @@ protected Node(final Environment initialEnvironment, final Transport transport = networkModule.getTransportSupplier().get(); Set taskHeaders = Stream.concat( pluginsService.filterPlugins(ActionPlugin.class).stream().flatMap(p -> p.getTaskHeaders().stream()), - Stream.of(Task.X_OPAQUE_ID) + Stream.of(Task.X_OPAQUE_ID, Task.TRACE_ID) ).collect(Collectors.toSet()); final TransportService transportService = newTransportService(settings, transport, threadPool, networkModule.getTransportInterceptor(), localNodeFactory, settingsModule.getClusterSettings(), taskHeaders); diff --git a/server/src/main/java/org/elasticsearch/rest/RestController.java b/server/src/main/java/org/elasticsearch/rest/RestController.java index 63bd4f3640b32..467881c1e45b5 100644 --- a/server/src/main/java/org/elasticsearch/rest/RestController.java +++ b/server/src/main/java/org/elasticsearch/rest/RestController.java @@ -28,6 +28,7 @@ import org.elasticsearch.http.HttpServerTransport; import org.elasticsearch.indices.breaker.CircuitBreakerService; import org.elasticsearch.rest.RestHandler.Route; +import org.elasticsearch.tasks.Task; import org.elasticsearch.usage.UsageService; import java.io.ByteArrayOutputStream; @@ -309,29 +310,15 @@ private void sendContentTypeErrorMessage(@Nullable List contentTypeHeade } private void tryAllHandlers(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) throws Exception { - for (final RestHeaderDefinition restHeader : headersToCopy) { - final String name = restHeader.getName(); - final List headerValues = request.getAllHeaderValues(name); - if (headerValues != null && headerValues.isEmpty() == false) { - final List distinctHeaderValues = headerValues.stream().distinct().collect(Collectors.toList()); - if (restHeader.isMultiValueAllowed() == false && distinctHeaderValues.size() > 1) { - channel.sendResponse( - BytesRestResponse. - createSimpleErrorResponse(channel, BAD_REQUEST, "multiple values for single-valued header [" + name + "].")); - return; - } else { - threadContext.putHeader(name, String.join(",", distinctHeaderValues)); - } - } - } - // error_trace cannot be used when we disable detailed errors - // we consume the error_trace parameter first to ensure that it is always consumed - if (request.paramAsBoolean("error_trace", false) && channel.detailedErrorsEnabled() == false) { - channel.sendResponse( - BytesRestResponse.createSimpleErrorResponse(channel, BAD_REQUEST, "error traces in responses are disabled.")); + try { + copyRestHeaders(request, threadContext); + validateErrorTrace(request, channel); + } catch (IllegalArgumentException e) { + channel.sendResponse(BytesRestResponse.createSimpleErrorResponse(channel, BAD_REQUEST, e.getMessage())); return; } + final String rawPath = request.rawPath(); final String uri = request.uri(); final RestRequest.Method requestMethod; @@ -365,6 +352,34 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel handleBadRequest(uri, requestMethod, channel); } + private void validateErrorTrace(RestRequest request, RestChannel channel) { + // error_trace cannot be used when we disable detailed errors + // we consume the error_trace parameter first to ensure that it is always consumed + if (request.paramAsBoolean("error_trace", false) && channel.detailedErrorsEnabled() == false) { + throw new IllegalArgumentException("error traces in responses are disabled."); + } + } + + private void copyRestHeaders(RestRequest request, ThreadContext threadContext) throws IOException { + for (final RestHeaderDefinition restHeader : headersToCopy) { + final String name = restHeader.getName(); + final List headerValues = request.getAllHeaderValues(name); + if (headerValues != null && headerValues.isEmpty() == false) { + final List distinctHeaderValues = headerValues.stream().distinct().collect(Collectors.toList()); + if (restHeader.isMultiValueAllowed() == false && distinctHeaderValues.size() > 1) { + throw new IllegalArgumentException("multiple values for single-valued header [" + name + "]."); + } else if (name.equals(Task.TRACE_PARENT)) { + String traceparent = distinctHeaderValues.get(0); + if (traceparent.length() >= 55) { + threadContext.putHeader(Task.TRACE_ID, traceparent.substring(3, 35)); + } + } else { + threadContext.putHeader(name, String.join(",", distinctHeaderValues)); + } + } + } + } + Iterator getAllHandlers(@Nullable Map requestParamsRef, String rawPath) { final Supplier> paramsSupplier; if (requestParamsRef == null) { diff --git a/server/src/main/java/org/elasticsearch/tasks/Task.java b/server/src/main/java/org/elasticsearch/tasks/Task.java index 9b0a77f5be9d3..b65669a110758 100644 --- a/server/src/main/java/org/elasticsearch/tasks/Task.java +++ b/server/src/main/java/org/elasticsearch/tasks/Task.java @@ -28,6 +28,19 @@ public class Task { */ public static final String X_OPAQUE_ID = "X-Opaque-Id"; + /** + * The request header which is contained in HTTP request. We parse trace.id from it and store it in thread context. + * TRACE_PARENT once parsed in RestController.tryAllHandler is not preserved + * has to be declared as a header copied over from http request. + */ + public static final String TRACE_PARENT = "traceparent"; + + /** + * Parsed part of traceparent. It is stored in thread context and emitted in logs. + * Has to be declared as a header copied over for tasks. + */ + public static final String TRACE_ID = "trace.id"; + private final long id; private final String type; diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index bd8375440d90c..961c3821cee64 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -32,6 +32,7 @@ import org.elasticsearch.http.HttpStats; import org.elasticsearch.indices.breaker.HierarchyCircuitBreakerService; import org.elasticsearch.rest.RestHandler.Route; +import org.elasticsearch.tasks.Task; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.client.NoOpNodeClient; import org.elasticsearch.test.rest.FakeRestRequest; @@ -162,6 +163,38 @@ public void testRequestWithDisallowedMultiValuedHeader() { assertTrue(channel.getSendResponseCalled()); } + public void testTraceParentAndTraceId() throws Exception { + final ThreadContext threadContext = client.threadPool().getThreadContext(); + Set headers = new HashSet<>(Arrays.asList(new RestHeaderDefinition(Task.TRACE_PARENT, false))); + final RestController restController = new RestController(headers, null, null, circuitBreakerService, usageService); + Map> restHeaders = new HashMap<>(); + restHeaders.put(Task.TRACE_PARENT, Collections.singletonList("00-0af7651916cd43dd8448eb211c80319c-b7ad6b7169203331-01")); + RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders(restHeaders).build(); + final RestController spyRestController = spy(restController); + when(spyRestController.getAllHandlers(null, fakeRequest.rawPath())) + .thenReturn(new Iterator() { + @Override + public boolean hasNext() { + return false; + } + + @Override + public MethodHandlers next() { + return new MethodHandlers("/") + .addMethod(GET, (request, channel, client) -> { + assertEquals("0af7651916cd43dd8448eb211c80319c", threadContext.getHeader(Task.TRACE_ID)); + assertNull(threadContext.getHeader(Task.TRACE_PARENT)); + }); + } + }); + AssertingChannel channel = new AssertingChannel(fakeRequest, false, RestStatus.BAD_REQUEST); + restController.dispatchRequest(fakeRequest, channel, threadContext); + // the rest controller relies on the caller to stash the context, so we should expect these values here as we didn't stash the + // context in this test + assertEquals("0af7651916cd43dd8448eb211c80319c", threadContext.getHeader(Task.TRACE_ID)); + assertNull(threadContext.getHeader(Task.TRACE_PARENT)); + } + public void testRequestWithDisallowedMultiValuedHeaderButSameValues() { final ThreadContext threadContext = client.threadPool().getThreadContext(); Set headers = new HashSet<>(Arrays.asList(new RestHeaderDefinition("header.1", true), From 401f8eadc6a94472d88c6f1573b61d963500b112 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Wed, 14 Jul 2021 14:27:38 +0200 Subject: [PATCH 2/4] fix tests --- distribution/src/config/log4j2.properties | 2 +- .../elasticsearch/common/logging/JsonLoggerTests.java | 10 ++++------ .../common/logging/json_layout/log4j2.properties | 4 ++-- .../org/elasticsearch/common/logging/ESJsonLayout.java | 1 + .../elasticsearch/common/logging/HeaderWarning.java | 8 ++++++++ .../elasticsearch/common/logging/TraceIdConverter.java | 2 +- 6 files changed, 17 insertions(+), 10 deletions(-) diff --git a/distribution/src/config/log4j2.properties b/distribution/src/config/log4j2.properties index a11775c314d85..c57af926618ac 100644 --- a/distribution/src/config/log4j2.properties +++ b/distribution/src/config/log4j2.properties @@ -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 appender.deprecation_rolling.filter.rate_limit.type = RateLimitingFilter appender.deprecation_rolling.filePattern = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_deprecation-%i.json.gz diff --git a/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java b/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java index f065fb27a1bd8..c864d2125f159 100644 --- a/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java +++ b/qa/logging-config/src/test/java/org/elasticsearch/common/logging/JsonLoggerTests.java @@ -107,8 +107,7 @@ public void testParseFieldEmittingDeprecatedLogs() throws Exception { hasEntry("message", "Deprecated field [deprecated_name] used, expected [new_name] instead"), hasEntry(DeprecatedMessage.KEY_FIELD_NAME, "deprecated_field_deprecated_name"), hasEntry(DeprecatedMessage.X_OPAQUE_ID_FIELD_NAME, "someId"), - hasEntry(Task.TRACE_ID, "someTraceId"), - hasEntry("elasticsearch.event.category", "api") + hasEntry(Task.TRACE_ID, "someTraceId") ), allOf( hasEntry("type", "deprecation.elasticsearch"), @@ -119,8 +118,7 @@ public void testParseFieldEmittingDeprecatedLogs() throws Exception { hasEntry("message", "Deprecated field [deprecated_name2] used, expected [new_name] instead"), hasEntry(DeprecatedMessage.KEY_FIELD_NAME, "deprecated_field_deprecated_name2"), hasEntry(DeprecatedMessage.X_OPAQUE_ID_FIELD_NAME, "someId"), - hasEntry(Task.TRACE_ID, "someTraceId"), - hasEntry("elasticsearch.event.category", "api") + hasEntry(Task.TRACE_ID, "someTraceId") ) ) @@ -157,8 +155,8 @@ public void testDeprecatedMessage() throws Exception { hasEntry("cluster.name", "elasticsearch"), hasEntry("node.name", "sample-name"), hasEntry("message", "deprecated message1"), - hasEntry(DeprecatedMessage.KEY_FIELD_NAME, "a key"), - not(hasKey(DeprecatedMessage.X_OPAQUE_ID_FIELD_NAME)) + hasEntry(DeprecatedMessage.KEY_FIELD_NAME, "someKey"), + hasEntry("x-opaque-id", "someId") ) ) ); diff --git a/qa/logging-config/src/test/resources/org/elasticsearch/common/logging/json_layout/log4j2.properties b/qa/logging-config/src/test/resources/org/elasticsearch/common/logging/json_layout/log4j2.properties index 5653a9e22a29a..bb16cf3f90e7f 100644 --- a/qa/logging-config/src/test/resources/org/elasticsearch/common/logging/json_layout/log4j2.properties +++ b/qa/logging-config/src/test/resources/org/elasticsearch/common/logging/json_layout/log4j2.properties @@ -14,7 +14,7 @@ appender.deprecated.name = deprecated appender.deprecated.fileName = ${sys:es.logs.base_path}${sys:file.separator}${sys:es.logs.cluster_name}_deprecated.json appender.deprecated.layout.type = ESJsonLayout appender.deprecated.layout.type_name = deprecation.elasticsearch -appender.deprecated.layout.esmessagefields = x-opaque-id +appender.deprecated.layout.esmessagefields = x-opaque-id,key appender.deprecated.filter.rate_limit.type = RateLimitingFilter appender.deprecation_rolling_old.type = File @@ -28,7 +28,7 @@ appender.deprecatedconsole.type = Console appender.deprecatedconsole.name = deprecatedconsole appender.deprecatedconsole.layout.type = ESJsonLayout appender.deprecatedconsole.layout.type_name = deprecation.elasticsearch -appender.deprecatedconsole.layout.esmessagefields = x-opaque-id +appender.deprecatedconsole.layout.esmessagefields = x-opaque-id,key appender.deprecatedconsole.filter.rate_limit.type = RateLimitingFilter appender.index_search_slowlog_rolling.type = File diff --git a/server/src/main/java/org/elasticsearch/common/logging/ESJsonLayout.java b/server/src/main/java/org/elasticsearch/common/logging/ESJsonLayout.java index 5532809f05c88..ab1201e583090 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/ESJsonLayout.java +++ b/server/src/main/java/org/elasticsearch/common/logging/ESJsonLayout.java @@ -110,6 +110,7 @@ private String createPattern(Map map, Set esMessageField separator = ", "; } sb.append(notEmpty(", %node_and_cluster_id ")); + sb.append(notEmpty(", %trace_id ")); sb.append("%exceptionAsJson "); sb.append("}"); sb.append(System.lineSeparator()); diff --git a/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java b/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java index 1b76e7bbcc76a..681c59723bcef 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java +++ b/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java @@ -310,6 +310,14 @@ public static String getXOpaqueId() { .orElse(""); } + public static String getTraceId() { + return THREAD_CONTEXT.stream() + .filter(t -> t.getHeader(Task.TRACE_ID) != null) + .findFirst() + .map(t -> t.getHeader(Task.TRACE_ID)) + .orElse(""); + } + public static void addWarning(String message, Object... params) { addWarning(THREAD_CONTEXT, message, params); } diff --git a/server/src/main/java/org/elasticsearch/common/logging/TraceIdConverter.java b/server/src/main/java/org/elasticsearch/common/logging/TraceIdConverter.java index d57fa4a0ae1bd..24af16647403e 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/TraceIdConverter.java +++ b/server/src/main/java/org/elasticsearch/common/logging/TraceIdConverter.java @@ -52,7 +52,7 @@ public static String getTraceId() { public void format(LogEvent event, StringBuilder toAppendTo) { String traceId = getTraceId(); if (traceId != null) { - toAppendTo.append(traceId); + toAppendTo.append("\"trace.id\": \"" + traceId + "\""); } } From d1796533137b303ed9381cd1d39267bb9da8b9d3 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Wed, 14 Jul 2021 15:11:42 +0200 Subject: [PATCH 3/4] es json test --- .../org/elasticsearch/common/logging/ESJsonLayoutTests.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/qa/logging-config/src/test/java/org/elasticsearch/common/logging/ESJsonLayoutTests.java b/qa/logging-config/src/test/java/org/elasticsearch/common/logging/ESJsonLayoutTests.java index 36e9624d3f991..c2b104ee851dd 100644 --- a/qa/logging-config/src/test/java/org/elasticsearch/common/logging/ESJsonLayoutTests.java +++ b/qa/logging-config/src/test/java/org/elasticsearch/common/logging/ESJsonLayoutTests.java @@ -40,6 +40,7 @@ public void testLayout() { "\"node.name\": \"%node_name\", " + "\"message\": \"%notEmpty{%enc{%marker}{JSON} }%enc{%.-10000m}{JSON}\"" + "%notEmpty{, %node_and_cluster_id }" + + "%notEmpty{, %trace_id }" + "%exceptionAsJson }" + System.lineSeparator())); } @@ -62,6 +63,7 @@ public void testLayoutWithAdditionalFields() { "%notEmpty{, \"x-opaque-id\": \"%ESMessageField{x-opaque-id}\"}" + "%notEmpty{, \"someOtherField\": \"%ESMessageField{someOtherField}\"}" + "%notEmpty{, %node_and_cluster_id }" + + "%notEmpty{, %trace_id }" + "%exceptionAsJson }" + System.lineSeparator())); } @@ -82,6 +84,7 @@ public void testLayoutWithAdditionalFieldOverride() { "\"node.name\": \"%node_name\"" + "%notEmpty{, \"message\": \"%ESMessageField{message}\"}" + "%notEmpty{, %node_and_cluster_id }" + + "%notEmpty{, %trace_id }" + "%exceptionAsJson }" + System.lineSeparator())); } } From 86013cc70ae72e92bcf0943c597db64cc224bfe9 Mon Sep 17 00:00:00 2001 From: pgomulka Date: Mon, 26 Jul 2021 12:05:51 +0200 Subject: [PATCH 4/4] unused method --- .../org/elasticsearch/common/logging/HeaderWarning.java | 8 -------- 1 file changed, 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java b/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java index 681c59723bcef..1b76e7bbcc76a 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java +++ b/server/src/main/java/org/elasticsearch/common/logging/HeaderWarning.java @@ -310,14 +310,6 @@ public static String getXOpaqueId() { .orElse(""); } - public static String getTraceId() { - return THREAD_CONTEXT.stream() - .filter(t -> t.getHeader(Task.TRACE_ID) != null) - .findFirst() - .map(t -> t.getHeader(Task.TRACE_ID)) - .orElse(""); - } - public static void addWarning(String message, Object... params) { addWarning(THREAD_CONTEXT, message, params); }