Add a PoC for OTel process context support #9472
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| package com.datadog.profiling.agent; | ||
|
|
||
| import datadog.libs.ddprof.DdprofLibraryLoader; | ||
| import datadog.trace.api.Config; | ||
| import datadog.trace.api.config.ProfilingConfig; | ||
| import datadog.trace.bootstrap.config.provider.ConfigProvider; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
|
||
| public final class ProcessContext { | ||
| private static final Logger log = LoggerFactory.getLogger(ProcessContext.class.getName()); | ||
|
|
||
| public static void register(ConfigProvider configProvider) { | ||
| if (configProvider.getBoolean( | ||
| ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED, | ||
| ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED_DEFAULT)) { | ||
| log.info("Registering process context for OTel profiler"); | ||
| DdprofLibraryLoader.OTelContextHolder holder = DdprofLibraryLoader.otelContext(); | ||
| Throwable err = holder.getReasonNotLoaded(); | ||
| if (err == null) { | ||
| Config cfg = Config.get(); | ||
| holder | ||
| .getComponent() | ||
| .setProcessContext( | ||
| cfg.getEnv(), | ||
| cfg.getHostName(), | ||
| cfg.getRuntimeId(), | ||
| cfg.getServiceName(), | ||
| cfg.getRuntimeVersion(), | ||
| cfg.getVersion()); | ||
| } else { | ||
| log.warn("Failed to register process context for OTel profiler", err); | ||
| } | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| package com.datadog.profiling.agent; | ||
|
|
||
| import static org.mockito.ArgumentMatchers.eq; | ||
| import static org.mockito.Mockito.mock; | ||
| import static org.mockito.Mockito.mockStatic; | ||
| import static org.mockito.Mockito.verify; | ||
| import static org.mockito.Mockito.when; | ||
|
|
||
| import com.datadoghq.profiler.OTelContext; | ||
| import datadog.libs.ddprof.DdprofLibraryLoader; | ||
| import datadog.trace.api.Config; | ||
| import datadog.trace.api.config.ProfilingConfig; | ||
| import datadog.trace.bootstrap.config.provider.ConfigProvider; | ||
| import org.junit.jupiter.api.Test; | ||
| import org.mockito.MockedStatic; | ||
|
|
||
| class ProcessContextTest { | ||
|
|
||
| @Test | ||
| void testRegisterSetsProcessContextValues() { | ||
| ConfigProvider configProvider = mock(ConfigProvider.class); | ||
| when(configProvider.getBoolean( | ||
| eq(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED), | ||
| eq(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED_DEFAULT))) | ||
| .thenReturn(true); | ||
|
|
||
| Config config = mock(Config.class); | ||
| when(config.getEnv()).thenReturn("test-env"); | ||
| when(config.getHostName()).thenReturn("test-host"); | ||
| when(config.getRuntimeId()).thenReturn("test-runtime-id"); | ||
| when(config.getServiceName()).thenReturn("test-service"); | ||
| when(config.getRuntimeVersion()).thenReturn("test-runtime-version"); | ||
| when(config.getVersion()).thenReturn("test-version"); | ||
|
|
||
| OTelContext otelContext = mock(OTelContext.class); | ||
| DdprofLibraryLoader.OTelContextHolder holder = | ||
| mock(DdprofLibraryLoader.OTelContextHolder.class); | ||
| when(holder.getReasonNotLoaded()).thenReturn(null); | ||
| when(holder.getComponent()).thenReturn(otelContext); | ||
|
|
||
| try (MockedStatic<Config> configMock = mockStatic(Config.class); | ||
| MockedStatic<DdprofLibraryLoader> ddprofMock = mockStatic(DdprofLibraryLoader.class)) { | ||
|
|
||
| configMock.when(Config::get).thenReturn(config); | ||
| ddprofMock.when(DdprofLibraryLoader::otelContext).thenReturn(holder); | ||
|
|
||
| ProcessContext.register(configProvider); | ||
|
|
||
| verify(otelContext) | ||
| .setProcessContext( | ||
| eq("test-env"), | ||
| eq("test-host"), | ||
| eq("test-runtime-id"), | ||
| eq("test-service"), | ||
| eq("test-runtime-version"), | ||
| eq("test-version")); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| void testRegisterSkipsWhenDisabled() { | ||
| ConfigProvider configProvider = mock(ConfigProvider.class); | ||
| when(configProvider.getBoolean( | ||
| eq(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED), | ||
| eq(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED_DEFAULT))) | ||
| .thenReturn(false); | ||
|
|
||
| DdprofLibraryLoader.OTelContextHolder holder = | ||
| mock(DdprofLibraryLoader.OTelContextHolder.class); | ||
|
|
||
| try (MockedStatic<DdprofLibraryLoader> ddprofMock = mockStatic(DdprofLibraryLoader.class)) { | ||
| ddprofMock.when(DdprofLibraryLoader::otelContext).thenReturn(holder); | ||
|
|
||
| ProcessContext.register(configProvider); | ||
|
|
||
| verify(holder, org.mockito.Mockito.never()).getReasonNotLoaded(); | ||
| verify(holder, org.mockito.Mockito.never()).getComponent(); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| void testRegisterSkipsByDefault() { | ||
| ConfigProvider configProvider = mock(ConfigProvider.class); | ||
| when(configProvider.getBoolean( | ||
| eq(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED), | ||
| eq(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED_DEFAULT))) | ||
| .thenReturn(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED_DEFAULT); | ||
|
|
||
| DdprofLibraryLoader.OTelContextHolder holder = | ||
| mock(DdprofLibraryLoader.OTelContextHolder.class); | ||
|
|
||
| try (MockedStatic<DdprofLibraryLoader> ddprofMock = mockStatic(DdprofLibraryLoader.class)) { | ||
| ddprofMock.when(DdprofLibraryLoader::otelContext).thenReturn(holder); | ||
|
|
||
| ProcessContext.register(configProvider); | ||
|
|
||
| verify(holder, org.mockito.Mockito.never()).getReasonNotLoaded(); | ||
| verify(holder, org.mockito.Mockito.never()).getComponent(); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| void testRegisterHandlesLibraryLoadFailure() { | ||
| ConfigProvider configProvider = mock(ConfigProvider.class); | ||
| when(configProvider.getBoolean( | ||
| eq(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED), | ||
| eq(ProfilingConfig.PROFILING_PROCESS_CONTEXT_ENABLED_DEFAULT))) | ||
| .thenReturn(true); | ||
|
|
||
| Throwable loadError = new RuntimeException("Library load failed"); | ||
| DdprofLibraryLoader.OTelContextHolder holder = | ||
| mock(DdprofLibraryLoader.OTelContextHolder.class); | ||
| when(holder.getReasonNotLoaded()).thenReturn(loadError); | ||
|
|
||
| try (MockedStatic<DdprofLibraryLoader> ddprofMock = mockStatic(DdprofLibraryLoader.class)) { | ||
| ddprofMock.when(DdprofLibraryLoader::otelContext).thenReturn(holder); | ||
|
|
||
| ProcessContext.register(configProvider); | ||
|
|
||
| verify(holder).getReasonNotLoaded(); | ||
| verify(holder, org.mockito.Mockito.never()).getComponent(); | ||
| } | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ | |
|
|
||
| import com.datadoghq.profiler.JVMAccess; | ||
| import com.datadoghq.profiler.JavaProfiler; | ||
| import com.datadoghq.profiler.OTelContext; | ||
| import datadog.trace.api.config.ProfilingConfig; | ||
| import datadog.trace.bootstrap.config.provider.ConfigProvider; | ||
| import datadog.trace.util.TempLocationManager; | ||
|
|
@@ -91,12 +92,25 @@ public JVMAccessHolder(Supplier<? extends ComponentHolder<JVMAccess>> initialize | |
| } | ||
| } | ||
|
|
||
| public static final class OTelContextHolder extends ComponentHolder<OTelContext> { | ||
| public OTelContextHolder(Supplier<? extends ComponentHolder<OTelContext>> initializer) { | ||
| super(initializer); | ||
| } | ||
|
|
||
| OTelContextHolder(OTelContext component, Throwable reasonNotLoaded) { | ||
| super(component, reasonNotLoaded); | ||
| } | ||
| } | ||
|
|
||
| private static final JavaProfilerHolder PROFILER_HOLDER = | ||
| new JavaProfilerHolder(DdprofLibraryLoader::initJavaProfiler); | ||
|
|
||
| private static final JVMAccessHolder JVM_ACCESS_HOLDER = | ||
| new JVMAccessHolder(DdprofLibraryLoader::initJVMAccess); | ||
|
|
||
| private static final OTelContextHolder OTEL_CONTEXT_HOLDER = | ||
| new OTelContextHolder(DdprofLibraryLoader::initOtelContext); | ||
|
|
||
| public static JavaProfilerHolder javaProfiler() { | ||
| return PROFILER_HOLDER; | ||
| } | ||
|
|
@@ -105,6 +119,10 @@ public static JVMAccessHolder jvmAccess() { | |
| return JVM_ACCESS_HOLDER; | ||
| } | ||
|
|
||
| public static OTelContextHolder otelContext() { | ||
| return OTEL_CONTEXT_HOLDER; | ||
| } | ||
|
|
||
| private static JavaProfilerHolder initJavaProfiler() { | ||
| JavaProfiler profiler; | ||
| Throwable reasonNotLoaded = null; | ||
|
|
@@ -144,6 +162,26 @@ private static JVMAccessHolder initJVMAccess() { | |
| return new JVMAccessHolder(jvmAccess, reasonNotLoaded.get()); | ||
| } | ||
|
|
||
| private static OTelContextHolder initOtelContext() { | ||
| ConfigProvider configProvider = ConfigProvider.getInstance(); | ||
| AtomicReference<Throwable> reasonNotLoaded = new AtomicReference<>(); | ||
| OTelContext otelContext = null; | ||
|
Contributor
There was a problem hiding this comment. ❔ question: Did they name it using the same name / semantic than their Context API? Aren't we be confused with the existing
Contributor
Author
There was a problem hiding this comment. This is a good question. Go went with 'process context' (DataDog/dd-trace-go#3937) - but, the long-term plan is to support both process-level as well as thread-level context transfer to the ebpf profiler. So, I decided to use This is all still in progress - if this is confusing we can change the epbf context propagator native wrapper class name to something else, before making this GA. |
||
| try { | ||
| String scratchDir = getScratchDir(configProvider); | ||
| otelContext = new OTelContext(null, scratchDir, reasonNotLoaded::set); | ||
| } catch (Throwable t) { | ||
| if (reasonNotLoaded.get() == null) { | ||
| reasonNotLoaded.set(t); | ||
| } else { | ||
| // if we already have a reason, don't overwrite it | ||
| // this can happen if the OTelContext constructor throws an exception | ||
| // and then the execute method throws another one | ||
| } | ||
| otelContext = null; | ||
| } | ||
| return new OTelContextHolder(otelContext, reasonNotLoaded.get()); | ||
| } | ||
|
|
||
| private static String getScratchDir(ConfigProvider configProvider) throws IOException { | ||
| String scratch = configProvider.getString(ProfilingConfig.PROFILING_DATADOG_PROFILER_SCRATCH); | ||
| if (scratch == null) { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
❔ question: Is there a reason to use
ConfigProviderinstead of creating an entry intoConfigfor the newPROFILING_PROCESS_CONTEXT_ENABLED? Is that because it's an experimental flag only that won't stay for long?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.
Yes. That's the main reason.
Also,
Configis a kind of abomination in its current form :) Is there anything particularly important I am missing by using theConfigProviderdirectly?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.
I tried answering the same question when looking at the flare reporter & came to the conclusion that anything profiling-specific should be dumped in
ProfilingConfig, while features that could/should affect other components are better off inConfig. I think given the experimental nature & profiling-specific func., we should keep it here for now.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.
Not really. Telemetry and config inversion will still work using ConfigProvider.
It might be a bit harder (not even sure) for newcomers to find out where is config is setup if it's handled on the side.
We really failed at providing useful / helpful config API so that's totally fine as it is right now. Maybe at some point we should advocate for product to create their own Config when it does not interact with the others.