[pkg/otlp] Backport changes from Datadog exporter #12452
Conversation
mx-psi
added
changelog/no-changelog
team/agent-apm
| // InvalidKind is an invalid kind. It is the zero value of Kind. | ||
| InvalidKind Kind = "" |
There was a problem hiding this comment.
I'd use the Go-style (see https://pkg.go.dev/net/http constants for Status* as one example) way of declaring constants representative of a type, and that is prefixing them with the type name (or some form of it), e.g.:
KindInvalidKindHostnameKindAWSECSFargate
There was a problem hiding this comment.
Since this PR is a backport and this would affect the public API, I will do this separately; I created AP-1722 to track this
| AWSECSFargateKind Kind = "task_arn" | ||
| ) | ||
|
|
||
| // Source represents a telemetry source. |
There was a problem hiding this comment.
Is this what we'd call a resource in OpenTelemetry? If so, wouldn't that be better? I'm still a newbie here
There was a problem hiding this comment.
Kinda, it's the Datadog-version of a resource. The goal is to have a single datatype that can represent hosts and serverless functions. Previously we had the RunningTagFromAttributes and HostnameFromAttributes, but since something can't be considered both a host and serverless, this simplifies the implementation a bit.
| // Provider identifies a source. | ||
| type Provider interface { |
There was a problem hiding this comment.
This is a nice opportunity to call it Sourcer, which is very standard for interfaces having only one function (calling them the name of that function + the *er suffix). From a contextual perspective source.Provider is nicer than source.Sourcer, but the latter ain't that bad either.
| type noSourceProvider struct{} | ||
|
|
||
| func (*noSourceProvider) Source(context.Context) (source.Source, error) { | ||
| return source.Source{Kind: source.HostnameKind, Identifier: ""}, nil |
There was a problem hiding this comment.
Do we need a source.KindUnknown type specifically for this?
There was a problem hiding this comment.
I think the right approach is to enforce passing the sourcer as a required argument in translator.New, to avoid this whole thing. I will do this together with the other changes on a future PR, since it affects public API
* [pkg/otlp/model] Backport changes from Datadog exporter * Adapt serializer and trace OTLP code to new changes * Address comments on Kubernetes code * [pkg/otlp/model] Rename to unknown source * [pkg/trace] Address comments * update ReceiveResourceSpans comment
What does this PR do?
Backports changes on the
pkg/otlp/modeldone in open-telemetry/opentelemetry-collector-contrib#11030 and adapts code to work with it.In particular, the concept of
Sourceis introduced. A source uniquely identifies where a piece of telemetry data comes from. It can either be a host or a serverless task/function (currently only ECS Fargate).Motivation
Keep codebases in sync. Allow open-telemetry/opentelemetry-collector-contrib#9693 to be merged.
Additional Notes
This does not have any user-facing changes. The code is split into two commits for easier review:
pkg/otlp/model.pkg/otlp/modelReviewer's Checklist
Triagemilestone is set.major_changelabel if your change either has a major impact on the code base, is impacting multiple teams or is changing important well-established internals of the Agent. This label will be use during QA to make sure each team pay extra attention to the changed behavior. For any customer facing change use a releasenote.changelog/no-changeloglabel has been applied.qa/skip-qalabel is not applied.team/..label has been applied, indicating the team(s) that should QA this change.need-change/operatorandneed-change/helmlabels have been applied.