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 service.name to MDC #9297

Open
cleverchuk opened this issue Aug 24, 2023 · 10 comments
Open

Add service.name to MDC #9297

cleverchuk opened this issue Aug 24, 2023 · 10 comments
Assignees
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request

Comments

@cleverchuk
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Currently, trace_id, trace_flags and span_id are added to MDC, however the service.name is not. This attribute is needed as well to tie the logs to the service that generated the logs. The current solution only allows tying them to a trace.

Describe the solution you'd like
Add service.name to the MDC so that it can be accessible to loggers.

Describe alternatives you've considered
Accept logs via the OTLP. This assumes that the system speaks OTLP natively which is not the case.

Additional context
Ability to correlate logs exported via non-OTLP to service. The logs are exported by reading the log file.

@cleverchuk cleverchuk added the enhancement New feature or request label Aug 24, 2023
@mateuszrzeszutek mateuszrzeszutek added the needs triage New issue that requires triage label Aug 25, 2023
@mateuszrzeszutek
Copy link
Member

This is something that is currently (sort of) available in the Splunk distro -- it's done in a bit hacky way, where we copy resource attributes over to system properties: https://github.com/signalfx/splunk-otel-java/blob/main/custom/src/main/java/com/splunk/opentelemetry/resource/ResourceAttributesToSystemProperties.java

@open-telemetry/java-instrumentation-maintainers we wouldn't mind upstreaming feature over to this repo, if there's a broader interest here. (Of course, not as a sysprop hack: I believe if we were to do that we would add configuration setting for a list of resource attribute keys that are to be captured as part of MDC/ThreadContext)

@trask
Copy link
Member

trask commented Aug 25, 2023

add configuration setting for a list of resource attribute keys that are to be captured as part of MDC/ThreadContext

sgtm 👍

@trask trask added contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome and removed needs triage New issue that requires triage labels Aug 25, 2023
@cleverchuk
Copy link
Contributor Author

I'll like to take this one.

@cleverchuk
Copy link
Contributor Author

@mateuszrzeszutek Where would be a good place to add this implementation? It looks like the implementation will need to be able to access the Resource object as seen by the user code.

@mateuszrzeszutek
Copy link
Member

Where would be a good place to add this implementation?

To the MDC/ThreadContext modules, e.g. logback-mdc-1.0, log4j-context-data-2.7

It looks like the implementation will need to be able to access the Resource object as seen by the user code.

Yep -- I think we'll need to add a new bootstrap module that'll include a class holding the resource Attributes in a static var; initialized somewhere in the AgentInstaller. The logging modules could then get the configured attributes from the bootstrap class.

@cleverchuk
Copy link
Contributor Author

@mateuszrzeszutek Struggling to find a way to access the Resource. This PR removed a clean way to access it. It seems like an option might be to make a resource customizer and add it here. However that looks like it's meant to be a user interface only and the attributes cached at that point won't contain any user customization. How should I proceed?

@laurit
Copy link
Contributor

laurit commented Oct 10, 2023

@cleverchuk In javaagent-tooling create a class called SdkAutoconfigureAccess in io.opentelemetry.sdk.autoconfigure.

package io.opentelemetry.sdk.autoconfigure;

import io.opentelemetry.sdk.resources.Resource;

public final class SdkAutoconfigureAccess {

  public static Resource getResource(AutoConfiguredOpenTelemetrySdk sdk) {
    return sdk.getResource();
  }

  private SdkAutoconfigureAccess() {}
}

in AgentInstaller add something like

private static void setResourceAttributes(AutoConfiguredOpenTelemetrySdk autoConfiguredSdk) {
   Resource resource = SdkAutoconfigureAccess.getResource(autoConfiguredSdk);
   // add ResourceAttributesHolder to javaagent-bootstrap module. you'll probably have to convert Attributes -> Map<String, String> somewhere
   ResourceAttributesHolder.setAttributes(resource.getAttributes());
}

call this method in installBytebuddyAgent before calling BeforeAgentListeners (could be after also, just a general guideline to help you locate the correct place).
Now you have the attributes available in a class called ResourceAttributesHolder that is in boot loader and accessible from logger javaagent instrumentations. For library instrumentations you'll have to do this differently, but I think these can be ignored for now.

@cleverchuk
Copy link
Contributor Author

cleverchuk commented Oct 10, 2023

@laurit @mateuszrzeszutek
Do I need write access or make use of a fork to push a branch/PR?

@cleverchuk
Copy link
Contributor Author

It looks like it should be a fork

cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 10, 2023
…ation.mdc.resource-attributes` that will be used to configure `Resource` attributes to be added to logging map diagnostic context.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 10, 2023
…ation.mdc.resource-attributes` that will be used to configure `Resource` attributes to be added to logging map diagnostic context.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 10, 2023
…ation.mdc.resource-attributes` that will be used to configure `Resource` attributes to be added to logging map diagnostic context.
@cleverchuk
Copy link
Contributor Author

@laurit please can you take a look at the draft PR at your convenience. How do I make the failing tests pass?

cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 10, 2023
…ation.mdc.resource-attributes` that will be used to configure `Resource` attributes to be added to logging map diagnostic context.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 11, 2023
…ation.mdc.resource-attributes` that will be used to configure `Resource` attributes to be added to logging map diagnostic context.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 11, 2023
…ation.mdc.resource-attributes` that will be used to configure `Resource` attributes to be added to logging map diagnostic context.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 11, 2023
- use assertJ for unit test assertions
- use plural for method and field names
- add compile only dependency on `:javaagent-bootstrap` for `instrumentation:log4j:log4j-context-data:log4j-context-data-2.17:library-autoconfigure`
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 12, 2023
…ation.mdc.resource-attributes` that will be used to configure `Resource` attributes to be added to logging map diagnostic context.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 12, 2023
- use assertJ for unit test assertions
- use plural for method and field names
- add compile only dependency on `:javaagent-bootstrap` for `instrumentation:log4j:log4j-context-data:log4j-context-data-2.17:library-autoconfigure`
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 12, 2023
- use implementation dependency on `:javaagent-bootstrap`
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 16, 2023
- use compile dependency on `:javaagent-bootstrap` and do runtime check before use.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 16, 2023
- use compile dependency on `:javaagent-bootstrap` and do runtime check before use.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 17, 2023
- use compile dependency on `:javaagent-bootstrap` and do runtime check before use.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 17, 2023
…ation.mdc.resource-attributes` that will be used to configure `Resource` attributes to be added to logging map diagnostic context.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 17, 2023
- use assertJ for unit test assertions
- use plural for method and field names
- add compile only dependency on `:javaagent-bootstrap` for `instrumentation:log4j:log4j-context-data:log4j-context-data-2.17:library-autoconfigure`
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 17, 2023
- use implementation dependency on `:javaagent-bootstrap`
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 17, 2023
- use compile dependency on `:javaagent-bootstrap` and do runtime check before use.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 25, 2023
…ation.mdc.resource-attributes` that will be used to configure `Resource` attributes to be added to logging map diagnostic context.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 25, 2023
- use assertJ for unit test assertions
- use plural for method and field names
- add compile only dependency on `:javaagent-bootstrap` for `instrumentation:log4j:log4j-context-data:log4j-context-data-2.17:library-autoconfigure`
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 25, 2023
- use implementation dependency on `:javaagent-bootstrap`
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 25, 2023
- use compile dependency on `:javaagent-bootstrap` and do runtime check before use.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 25, 2023
- refactor constructor to static method
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 26, 2023
- move ConfiguredResourceAttributesHolder to javaagent-extension-api
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 26, 2023
…ation.mdc.resource-attributes` that will be used to configure `Resource` attributes to be added to logging map diagnostic context.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 26, 2023
- use assertJ for unit test assertions
- use plural for method and field names
- add compile only dependency on `:javaagent-bootstrap` for `instrumentation:log4j:log4j-context-data:log4j-context-data-2.17:library-autoconfigure`
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 26, 2023
- use implementation dependency on `:javaagent-bootstrap`
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 26, 2023
- use compile dependency on `:javaagent-bootstrap` and do runtime check before use.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 26, 2023
- refactor constructor to static method
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Oct 26, 2023
- move ConfiguredResourceAttributesHolder to javaagent-extension-api
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Nov 9, 2023
…ation.mdc.resource-attributes` that will be used to configure `Resource` attributes to be added to logging map diagnostic context.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Nov 9, 2023
- use assertJ for unit test assertions
- use plural for method and field names
- add compile only dependency on `:javaagent-bootstrap` for `instrumentation:log4j:log4j-context-data:log4j-context-data-2.17:library-autoconfigure`
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Nov 9, 2023
- use implementation dependency on `:javaagent-bootstrap`
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Nov 9, 2023
- use compile dependency on `:javaagent-bootstrap` and do runtime check before use.
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Nov 9, 2023
- refactor constructor to static method
cleverchuk added a commit to cleverchuk/opentelemetry-java-instrumentation that referenced this issue Nov 9, 2023
- move ConfiguredResourceAttributesHolder to javaagent-extension-api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution welcome Request makes sense, maintainers probably won't have time, contribution would be welcome enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants