RUM-15422: Support addEvent for OTel spans (logging events)#3377
RUM-15422: Support addEvent for OTel spans (logging events)#3377
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3377 +/- ##
===========================================
+ Coverage 71.49% 71.89% +0.40%
===========================================
Files 947 947
Lines 34919 34930 +11
Branches 5921 5806 -115
===========================================
+ Hits 24965 25111 +146
+ Misses 8290 8228 -62
+ Partials 1664 1591 -73
🚀 New features to boost your workflow:
|
| * Logs a set of attributes with a custom timestamp and associates them with the current [DatadogSpan]. | ||
| * | ||
| * @param attributes A map containing key-value pairs of attributes to be logged. | ||
| * @param timestampMs The timestamp in milliseconds since epoch at which the event occurred. |
There was a problem hiding this comment.
this argument is actually not used, is it expected?
There was a problem hiding this comment.
this parameter is being used in the real implementation. Here is just a default implementation to keep compatibility with the previous SDK version
There was a problem hiding this comment.
Eventually I changed it to be sent with an internal argument instead of the separate parameter because it could break binary compatibility on a java level
The `forge` parameter on the `fall back to timeProvider` test case was never referenced; kotlinc with -Werror failed the build because of the unused-parameter warning. Removing it lets the trace module's unit tests compile cleanly.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aafc48f4af
ℹ️ 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".
| if (value != null) fields.put(item.getKey().getKey(), value); | ||
| } | ||
| fields.put(DatadogTracingConstants.LogAttributes.MESSAGE, name); |
There was a problem hiding this comment.
Preserve attributes that use reserved log keys
When an OTel event includes an attribute whose key matches a span-log control key, this map stores it in the same namespace used by DatadogSpanLogger. In particular Attributes.of(stringKey("message"), value) is overwritten here by the event name, and keys such as status or _dd.timestamp_ms are later removed/interpreted as logger controls instead of being emitted as event attributes. Since OTel event attributes are arbitrary user data, route the event name/timestamp separately or escape these reserved keys before delegating so those attributes are not silently lost or repurposed.
Useful? React with 👍 / 👎.
What does this PR do?
Implements
addEventsupport forOtelSpan, routing OpenTelemetry span events to Datadog's logging infrastructure.logAttributes(attributes: Map<String, Any>)andlogAttributes(attributes: Map<String, Any>, timestampMs: Long)toDatadogSpaninterfaceaddEvent(String, Attributes)andaddEvent(String, Attributes, long, TimeUnit)inOtelSpan.java— converts OTelAttributesto a map and delegates toDatadogSpan.logAttributes()logAttributesinDatadogSpanAdapter, routing toDatadogSpanLoggerDatadogSpanLoggerto support optional customtimestampMsMotivation
OTel API requires
Span.addEvent()to forward span events as logs. Without this, events added via the OTel API were silently dropped.Additional Notes
--custom-rulesstep reports 210 pre-existing weighted issues (incronet,ApmNetworkInstrumentation, etc.) unrelated to this PRdd-sdk-android-sqldelight.apiupdated to reflect the new defaultlogAttributesimpl in theDatadogSpaninterfaceReview checklist (to be filled by reviewers)