Duplication of Resource Detectors with opentelemetry-go-contrib #1590
Comments
dackroyd
changed the title
|
I was looking into adding resource attributes for AWS and Kubernetes to applications running on that infrastructure. These could be detected at the application level, but that adds complexity and dependencies at that level which are likely undesirable. For example, using the OpenTelemetry-go-contrib implementation from the application to detect AWS attributes requires a dependency on the AWS SDK, even if the app doesn't use any of the AWS apis otherwise. That library is quite large, and OpenTelemetry-go-contrib doesn't have the resources to maintain an implementation that doesn't have this dependency: open-telemetry/opentelemetry-go-contrib#429 I think it is preferable not to have each application trying to detect these resource attributes themselves, but instead to have this done by the agent/collector for the infrastructure where the app is running. Other setups however may call for the application to do so, and there are other collectors (the Jaeger collector, or a customised one?) which would benefit from being able to reuse the same detection code, maintained in the one place (OpenTelemetry-go-contrib). |
|
@dackroyd completely agree, I think collector should be used for resource detection in general. |
Map and array attribute value types which were recently added to the specification had not been implemented in the collector yet. This PR implements across all tracing receivers and exporters. **Link to tracking Issue:** #1590 **Testing:** Added map and array attribute values to golden dataset. All unit and correctness tests continue to pass.
This adds an eks detector for Amazon EKS. It sets cloud.provider, cloud.infrastructure_service, and k8s.cluster.name. It's intended to be used in conjunction with ec2 detector. The eks detector should come first so that the more specific cloud.infrastructure_service value wins (see README.md). Note that this code is taken from https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/detectors/aws/eks. There is a goal to prevent duplication with go-contrib in #1590 and I started down that path but it will require more time than currently available as it needs to transform from different data models, deal with any differences in metadata between existing detectors, etc.
This adds an gke detector for GKE. It sets cloud.provider, cloud.infrastructure_service, and k8s.cluster.name. It's intended to be used in conjunction with gce detector. The gke detector should come first so that the more specific cloud.infrastructure_service value wins (see README.md). Note that this code is taken from https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/detectors/gcp. There is a goal to prevent duplication with go-contrib in #1590 and I started down that path but it will require more time than currently available as it needs to transform from different data models, deal with any differences in metadata between existing detectors, etc.
* [processor/resourcedetection] Add GKE detector This adds an gke detector for GKE. It sets cloud.provider, cloud.infrastructure_service, and k8s.cluster.name. It's intended to be used in conjunction with gce detector. The gke detector should come first so that the more specific cloud.infrastructure_service value wins (see README.md). Note that this code is taken from https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/detectors/gcp. There is a goal to prevent duplication with go-contrib in #1590 and I started down that path but it will require more time than currently available as it needs to transform from different data models, deal with any differences in metadata between existing detectors, etc. * add coverage for gce mock * review feedback * fix tests
|
I looked into this as part of adding EKS and GKE. There are some differences between detectors in go-contrib and here. e.g. they add container.id because they can determine what container the app is running in. If we were to include container.id we would put in the collector's own container id. You could configure the processor not to override existing attributes but it's also just not clear if collector should ever insert container.id (unless it's for self-metrics). go-contrib may also be more sensitive to dependencies being pulled in given their not using k8s client API in EKS for instance. (Though I haven't discussed it with them, I'm just inferring.) In collectors case it's less of a concern. Given the relatively small amount of code I'm not 100% sure it's worth the effort even though it sounds good in theory. |
This adds an eks detector for Amazon EKS. It sets cloud.provider, cloud.infrastructure_service, and k8s.cluster.name. It's intended to be used in conjunction with ec2 detector. The eks detector should come first so that the more specific cloud.infrastructure_service value wins (see README.md). Note that this code is taken from https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/detectors/aws/eks. There is a goal to prevent duplication with go-contrib in #1590 and I started down that path but it will require more time than currently available as it needs to transform from different data models, deal with any differences in metadata between existing detectors, etc.
* [processor/resourcedetection] Add GKE detector This adds an gke detector for GKE. It sets cloud.provider, cloud.infrastructure_service, and k8s.cluster.name. It's intended to be used in conjunction with gce detector. The gke detector should come first so that the more specific cloud.infrastructure_service value wins (see README.md). Note that this code is taken from https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/detectors/gcp. There is a goal to prevent duplication with go-contrib in #1590 and I started down that path but it will require more time than currently available as it needs to transform from different data models, deal with any differences in metadata between existing detectors, etc. * add coverage for gce mock * review feedback * fix tests
This adds an eks detector for Amazon EKS. It sets cloud.provider, cloud.infrastructure_service, and k8s.cluster.name. It's intended to be used in conjunction with ec2 detector. The eks detector should come first so that the more specific cloud.infrastructure_service value wins (see README.md). Note that this code is taken from https://github.com/open-telemetry/opentelemetry-go-contrib/blob/main/detectors/aws/eks. There is a goal to prevent duplication with go-contrib in #1590 and I started down that path but it will require more time than currently available as it needs to transform from different data models, deal with any differences in metadata between existing detectors, etc.
|
Hi @jrcamp @bogdandrutu can we untag |
|
This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping |
|
@Aneurysm9, is this something your team would be interested in working on? |
|
I'm not sure I see the need for, or value in, sharing implementations here. Would like to hear from other @open-telemetry/go-approvers, but I would currently lean toward closing this issue. |
|
Happy for the issue to be closed. Clear to me that there is a need for each to be implemented separately |
Is your feature request related to a problem? Please describe.
Resource detectors implemented in this repository duplicate those available in opentelemetry-go-contrib, or otherwise would benefit from being included there.
Describe the solution you'd like
Rather than maintaining independent implementations in both places, with inconsistencies and requiring maintenance in both places, resource detectors are implemented in opentelemetry-go-contrib, and used as a dependency.
This would also mean that those resource detectors are available when not using this collector (say the Jaeger variant, or another custom one?)
Describe alternatives you've considered
Having both projects maintain their own copies (as applies currently) likely provides more implementation flexibility, since not tied to changes being made in the other project and waiting on releases.
Additional context
AWS EC2: Collector Contrib / Go Contrib
AWS ECS: Collector Contrib / Go Contrib not implemented (submit?)
GCP GCE: Collector Contrib / Go Contrib
GCP GKE: Collector Contrib not implemented? / Go Contrib
Env: Collector Contrib / OTel Go Available OOTB with OTel Go
System (OS type, hostname): Collector Contrib / OTel Go/Go Contrib not implemented?
The text was updated successfully, but these errors were encountered: