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.

Sign up for GitHub

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

Jump to bottom

Allow configuring MDC key names for trace_id, span_id, trace_flags #11329

Merged

trask merged 22 commits into open-telemetry:main from AlchemyDing:mdc-key-names

Jun 10, 2024

Member

AlchemyDing commented May 10, 2024

resolve issue
continue with PR

AlchemyDing added 4 commits

May 10, 2024 10:58


          feat:Allow configuring MDC key names for trace_id, span_id, trace_flags

bf20110


          spotless apply

fc5827a

fix

7b7f877


          fix test

9022e4d

AlchemyDing requested a review from a team

May 10, 2024 06:36

Member Author

AlchemyDing commented May 10, 2024

continue in this pr @pavelvodrazka

This was referenced May 10, 2024

Allow configuring MDC key names for trace_id, span_id, trace_flags #9390

Closed

REQUEST: New membership for @AlchemyDing open-telemetry/community#2105

Closed

laurit reviewed

View reviewed changes

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/AgentInstaller.java Outdated Show resolved Hide resolved

laurit reviewed

View reviewed changes

...tion/log4j/log4j-context-data/log4j-context-data-2.17/library-autoconfigure/build.gradle.kts Outdated Show resolved Hide resolved

laurit reviewed

View reviewed changes

instrumentation/logback/logback-mdc-1.0/library/build.gradle.kts Outdated Show resolved Hide resolved

laurit reviewed

View reviewed changes

...y/src/main/java/io/opentelemetry/instrumentation/logback/mdc/v1_0/OpenTelemetryAppender.java Outdated Show resolved Hide resolved

AlchemyDing added 2 commits

May 15, 2024 10:40

fix

f1b8638


          add comments

6fd94a2

laurit reviewed

View reviewed changes

Contributor

laurit left a comment

could you also add tests

...y/src/main/java/io/opentelemetry/instrumentation/logback/mdc/v1_0/OpenTelemetryAppender.java Outdated Show resolved Hide resolved

.../opentelemetry/instrumentation/log4j/contextdata/v2_17/OpenTelemetryContextDataProvider.java Outdated Show resolved Hide resolved

...y/src/main/java/io/opentelemetry/instrumentation/logback/mdc/v1_0/OpenTelemetryAppender.java Outdated Show resolved Hide resolved

...y/src/main/java/io/opentelemetry/instrumentation/logback/mdc/v1_0/OpenTelemetryAppender.java Outdated Show resolved Hide resolved

...-extension-api/src/main/java/io/opentelemetry/javaagent/bootstrap/internal/CommonConfig.java Outdated Show resolved Hide resolved


          add tests

c430171

github-actions bot requested a review from theletterf

May 24, 2024 05:51

AlchemyDing added 3 commits

May 24, 2024 13:54


          spotlessApply

6eb4a7d


          Merge branch 'open-telemetry:main' into mdc-key-names

ea292d9

fix

fe8c72b

AlchemyDing requested a review from laurit

May 24, 2024 07:16

laurit reviewed

View reviewed changes

.../opentelemetry/instrumentation/log4j/contextdata/v2_17/OpenTelemetryContextDataProvider.java Outdated Show resolved Hide resolved

laurit reviewed

View reviewed changes

...mon/testing/src/main/java/io/opentelemetry/instrumentation/log4j/contextdata/Log4j2Test.java Show resolved Hide resolved

laurit reviewed

View reviewed changes

instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/README.md Outdated Show resolved Hide resolved

laurit reviewed

View reviewed changes

...y/src/main/java/io/opentelemetry/instrumentation/logback/mdc/v1_0/OpenTelemetryAppender.java Outdated Show resolved Hide resolved

laurit reviewed

View reviewed changes

...-extension-api/src/main/java/io/opentelemetry/javaagent/bootstrap/internal/CommonConfig.java Outdated Show resolved Hide resolved

fix

40bd82f

AlchemyDing requested a review from laurit

May 24, 2024 14:44

laurit reviewed

View reviewed changes

instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/build.gradle.kts Outdated Show resolved Hide resolved

laurit reviewed

View reviewed changes

...y/src/main/java/io/opentelemetry/instrumentation/logback/mdc/v1_0/OpenTelemetryAppender.java Outdated Show resolved Hide resolved

AlchemyDing added 3 commits

May 27, 2024 22:38

fix

43d8af5

fix

56c371e

fix

ced5625

laurit reviewed

View reviewed changes

instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/javaagent/build.gradle.kts Outdated

Comment on lines 29 to 33

-                    sources {
-                      java {
-                        setSrcDirs(listOf("src/test/java"))
-                      }
-                    }

Contributor

laurit May 29, 2024

You shouldn't have removed this. The intent here is that this test suite uses the same test sources as the main test. If you remove it then it will use src/testDisableThreadLocals/java as test sources, as that directory does not exist it won't run any tests.

Member Author

AlchemyDing May 30, 2024

Thank you for your answer

Contributor

laurit commented May 29, 2024

I think this is almost done. Only https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/log4j/log4j-context-data/log4j-context-data-2.17/library-autoconfigure/README.md and https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/logback/logback-mdc-1.0/library/README.md (seems to be missing the addBaggage flag too) need to be updated

fix

1455f0a

Member Author

AlchemyDing commented May 30, 2024

Why Build pull request / common / test0 (21, hotspot) (pull_request) has been cancelled

AlchemyDing added 4 commits

May 30, 2024 19:16


          update readme

adebffd


          update readme

13202e0

fix

e8d3fa0

fix

e20d5d1

laurit reviewed

View reviewed changes

instrumentation/logback/logback-mdc-1.0/library/README.md Outdated

Comment on lines 35 to 40

+              If the `otel.instrumentation.logback-mdc.add-baggage` system property (or the
+              `OTEL_INSTRUMENTATION_LOGBACK_MDC_ADD_BAGGAGE` environment variable) is set to `true`,
+              key/value pairs in [baggage](https://opentelemetry.io/docs/concepts/signals/baggage/) will be added to the context too.
+              - `baggage.<entry_name>`

Contributor

laurit May 31, 2024

in library instrumentation logback appender is configured via logback.xml not system properties or environment variables, see

opentelemetry-java-instrumentation/instrumentation/logback/logback-mdc-1.0/library/src/addBaggageTest/resources/logback.xml

Line 23 in b8c28df

<addBaggage>true</addBaggage>

laurit reviewed

View reviewed changes

instrumentation/logback/logback-mdc-1.0/library/README.md Outdated

Comment on lines 74 to 81

+              If you want to customize the names of these keys, you can set system property or environment variable:
+              | System property                                       | Environment variable                              |
+              |-------------------------------------------------------|---------------------------------------------------|
+              | `otel.instrumentation.common.logging.trace-id`        | `OTEL_INSTRUMENTATION_COMMON_LOGGING_TRACE_ID`    |
+              | `otel.instrumentation.common.logging.span-id`         | `OTEL_INSTRUMENTATION_COMMON_LOGGING_SPAN_ID`     |
+              | `otel.instrumentation.common.logging.trace-flags`     | `OTEL_INSTRUMENTATION_COMMON_LOGGING_TRACE_FLAGS` |

Contributor

laurit May 31, 2024

these are also configured in logback.xml

laurit reviewed

View reviewed changes

...rumentation/log4j/log4j-context-data/log4j-context-data-2.17/library-autoconfigure/README.md Outdated

               - `span_id`
               - `trace_flags`
+              If you want to customize the names of these keys, you can set system property or environment variable:

Contributor

laurit May 31, 2024

Suggested change

      
            If you want to customize the names of these keys, you can set system property or environment variable:
          
            These keys can be customized using the following system properties or environment variables:

AlchemyDing and others added 3 commits

May 31, 2024 18:42


          fix readme

abfd2f6


          fix readme

0f9211c


          small change to readme

eb4f021

laurit approved these changes

View reviewed changes

pavelvodrazka commented Jun 10, 2024

Hi, is this request planned to be merged / included in the next release?

trask merged commit b47aca9 into open-telemetry:main

53 checks passed

zeitlinger pushed a commit to zeitlinger/opentelemetry-java-instrumentation that referenced this pull request


          Allow configuring MDC key names for trace_id, span_id, trace_flags (o…

5083e86

…pen-telemetry#11329)

Co-authored-by: Lauri Tulmin <ltulmin@splunk.com>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet