Conversation
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 376c24d851
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
AlexeyKuznetsov-DD
left a comment
There was a problem hiding this comment.
Left several comments. My main concern that core classes changed, we need to double check on that.
| takesArgument( | ||
| 0, | ||
| named( | ||
| "com.microsoft.azure.functions.internal.spi.middleware.MiddlewareContext"))) |
There was a problem hiding this comment.
nit: "com.microsoft.azure.functions.internal.spi.middleware.MiddlewareContext" looks very long (and other similar strings), maybe make sense to create short consts MIDDLEWARE_CONTEXT ?
There was a problem hiding this comment.
Not sure it is worth since they only appear once. Also, the repository's instrumentation guidelines recommend keeping values inline in one-shot methods such as methodAdvice()
| private final Map<String, String> baggage; | ||
| private Baggage w3cBaggage; | ||
| private final int samplingPriority; | ||
| private int samplingPriority; |
There was a problem hiding this comment.
This is a change in a core class, betted to discuss with dd-trace-java such changes...
@dougqh WDYT?
There was a problem hiding this comment.
For context, I need to change the sampling priority here because the host runtime is .NET. We end up with sampling decisions and context that points to traces we have no access to. If we don't override the sampling we end up dropping all the spans unless we enable Opentelemetry in the Azure host runtime config. Telling customers to enable that is less than ideal since we want to have a native instrumentation. Open to other ideas, but we had to do this in all runtimes so far.
There was a problem hiding this comment.
... also check with @MilanGarnier because this may overlap with the consistent probability sampling RFC. Certainly this looks like something that needs careful consideration wrt. how it might interact with other efforts.
What Does This Do
Adds tracing support for Azure Durable Functions running on the Java worker:
FunctionExecutionMiddleware.Motivation
The existing Azure Functions instrumentation covers HTTP entry points, but Java Durable orchestrators and activities execute through worker middleware and were not represented in the distributed trace. This left the asynchronous Durable workflow invisible after the starter function scheduled an instance.
This instrumentation connects the Java Durable workflow to the originating trace and avoids emitting a duplicate orchestrator span for every successful replay.
Validation
./gradlew :dd-java-agent:instrumentation:azure-functions-worker-2.7:spotlessCheck./gradlew :dd-java-agent:instrumentation:azure-functions-worker-2.7:test./gradlew :dd-java-agent:instrumentation:azure-functions-worker-2.7:forkedTest./gradlew :dd-java-agent:instrumentation:azure-functions-worker-2.7:latestDepTest./gradlew :dd-trace-core:test --tests datadog.trace.core.propagation.ExtractedContextTest./gradlew :dd-java-agent:shadowJar :dd-java-agent:verifyAgentJarContents :dd-java-agent:verifyAgentJarIntegrationsNo error
With error
Additional Notes
2.7+and was validated withdurabletask-azure-functions1.0.1.Contributor Checklist
type: featureandinst: azure-functionslabels.CODEOWNERS.