[processor/resourcedetection] Add Amazon EKS detector #2820
Conversation
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.
Codecov Report
@@ Coverage Diff @@
## main #2820 +/- ##
==========================================
- Coverage 91.58% 91.44% -0.15%
==========================================
Files 452 453 +1
Lines 22222 22289 +67
==========================================
+ Hits 20353 20383 +30
- Misses 1397 1430 +33
- Partials 472 476 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
|
||
| * cloud.provider ("aws") | ||
| * cloud.infrastructure_service ("aws_eks") | ||
| * k8s.cluster.name (name of the EKS cluster) |
There was a problem hiding this comment.
For some reason I thought k8s doesn't have the concept of a cluster name but I see it in the spec, nice
Is now a good time to add arn? If not no worries
There was a problem hiding this comment.
Would rather do separately.
| } | ||
|
|
||
| // Detect returns a Resource describing the Amazon EKS environment being run in. | ||
| func (detector *Detector) Detect(ctx context.Context) (pdata.Resource, error) { |
There was a problem hiding this comment.
This code looks familiar, maybe ported from Java? I think for the collector since we already use the k8s client library elsewhere, we can just use it here instead of writing all this fetch logic.
There was a problem hiding this comment.
Ah reread PR description and see it's copied from the Go SDK - but yeah in the collector I would probably expect this to just use the client library. Otherwise if the code can be deduped that sounds good too though.
There was a problem hiding this comment.
Can you clarify your plan here? I think it could be easier to just use the k8s client library to fix all of @tigrannajaryan's comments
There was a problem hiding this comment.
Yeah I'm assuming go-contrib didn't use k8s API to avoid dragging in all of k8s dependencies. I'm reluctant to add it since it will add more friction if we try to upstream it. However, I don't think it's completely unreasonable to say go-contrib and our use cases are different enough that they will just have separate implementations and each does the best for their particular case.
Unless you have strong objections I say let's go with a minimally fixed up version in the hopes of consolidating with go-contrib.
processor/resourcedetectionprocessor/internal/aws/eks/detector.go
Outdated
Show resolved
Hide resolved
processor/resourcedetectionprocessor/internal/aws/eks/detector.go
Outdated
Show resolved
Hide resolved
processor/resourcedetectionprocessor/internal/aws/eks/detector.go
Outdated
Show resolved
Hide resolved
processor/resourcedetectionprocessor/internal/aws/eks/detector.go
Outdated
Show resolved
Hide resolved
processor/resourcedetectionprocessor/internal/aws/eks/detector.go
Outdated
Show resolved
Hide resolved
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 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.