[processor/datadogsemantics] Make defaults explicit and add test #39596
Conversation
github-actions
bot
requested review from
ankitpatel96,
dineshg13,
jackgopack4 and
jade-guiton-dd
There was a problem hiding this comment.
I would prefer if each attribute followed the same pattern as much as possible. If there is a way you can either set service variable explicitly prior to calling GetOTelService, or leave a comment above those lines explaining what service gets set to by default, that would be appreciated
| if err = tp.insertAttrIfMissingOrShouldOverride(rattr, "datadog.service", service); err != nil { | ||
| return ptrace.Traces{}, err | ||
| } | ||
| if err = tp.insertAttrIfMissingOrShouldOverride(rattr, "datadog.service", traceutil.GetOTelService(otelres, true)); err != nil { |
There was a problem hiding this comment.
am I understanding correctly that this change, if otelres is blank, then traceutil.GetOTelService will return "otlpresourcenoservicename" and that's what datadog.service will be set to?
https://github.com/DataDog/datadog-agent/blob/0d44e61fa623ef2572ba8d7fbe30e2632540d8aa/pkg/trace/traceutil/otel_util.go#L276
Worth adding a link to the function or noting the default setting in the comments here?
There was a problem hiding this comment.
alternatively, you could always make that an exportable constant in pkg/trace/traceutil and set service equal to the value before calling GetOTelService.
I just think we should try to have the pattern for each attribute follow a similar pattern to the maximum extent practical, so the code is easy to understand later
There was a problem hiding this comment.
Agreed, I added a comment explaining the default
Description
Explicitly set defaults for each field - whatever the Datadog backend would populate if you send the field as blank; e.g., if we send a blank env, it would show up in the UI as the string
default, so we should surface that to the user with this processor. Also, populatedatadog.*fields even if they would be blank (this includes overriding them even if the result would be blank if theoverrideIncomingDatadogFieldsoption is set, for consistency)Link to tracking issue
Fixes
Testing
Documentation