Add metrics support to k8s processor #358
Conversation
Kubernetes processor can be used to enrich not only spans but metrics with k8s metadata. In order to find a source of the metrics we take a different approach then for spans. The processor looks at "host.hostname" resource attribute which is set by prometheus receiver and some metrics instrumentation libraries to an originated IP address. That IP address is used by k8s processor to lookup for a k8s pod and fetch its metadata. The processor uses the old-style metrics data model since the new metric model is not available in contrib yet. But it must be easy to switch over later once we wave it the new metrics model support.
Codecov Report
@@ Coverage Diff @@
## master #358 +/- ##
==========================================
+ Coverage 83.15% 83.22% +0.07%
==========================================
Files 170 170
Lines 9116 9160 +44
==========================================
+ Hits 7580 7623 +43
Misses 1217 1217
- Partials 319 320 +1
Continue to review full report at Codecov.
|
| for _, opt := range options { | ||
| if err := opt(kp); err != nil { | ||
| return nil, err | ||
| } | ||
| } | ||
| err := kp.initKubeClient(logger, kubeClient) |
There was a problem hiding this comment.
nit: I feel it is slightly better if the function creates a client instance and returns it leaving it to the called to set it to a field. Not a big deal though in this case.
There was a problem hiding this comment.
I tried this way, but it seems adding more unnecessary complexity making this code reuse case not very useful.
|
|
||
| // Don't invoke any k8s client functionality in passthrough mode. | ||
| // Just tag the IP and forward the batch. | ||
| if kp.passthroughMode || podIP == "" { |
There was a problem hiding this comment.
nit: this is a very simple and obvious case but I still feel we should separate out these two condition in their own if statements. I feel that reads better especially with the comment above the line.
There was a problem hiding this comment.
Agreed. Pushed as another PR
| } | ||
| } | ||
|
|
||
| return kp.nextMetricsConsumer.ConsumeMetrics(ctx, metrics) |
There was a problem hiding this comment.
The loop above modifies mds and here we pass metrics. It is not apparent that modifying mds changes metrics (does it?). I am not sure if anything can be done to make it clear. We probably just need to wait until internal metric data type is available and get rid of the conversion.
There was a problem hiding this comment.
yes, it does. it operates on pointers https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/358/files/141277673a43b49fba34c48cef1cae2a45780a43#diff-325890f0a975205128a62dd76a8392f9R188 to have the modification applied.
|
I was just about to push few tests to get to 95% coverage and address nits :) |
#361) It addresses comment #358 (comment)
It's the tests part missed to get 95% percent coverage in #358.
Kubernetes processor can be used to enrich not only spans but metrics with k8s metadata. In order to find a source of the metrics we take a different approach then for spans. The processor looks at "host.hostname" resource attribute which is set by prometheus receiver and some metrics instrumentation libraries to an originated IP address. That IP address is used by k8s processor to lookup for a k8s pod and fetch its metadata. The processor uses the old-style metrics data model since the new metric model is not available in contrib yet. But it must be easy to switch over later once we wave it the new metrics model support.
It's the tests part missed to get 95% percent coverage in open-telemetry#358.
Bumps [k8s.io/client-go](https://github.com/kubernetes/client-go) from 0.23.2 to 0.23.3. - [Release notes](https://github.com/kubernetes/client-go/releases) - [Changelog](https://github.com/kubernetes/client-go/blob/master/CHANGELOG.md) - [Commits](kubernetes/client-go@v0.23.2...v0.23.3) --- updated-dependencies: - dependency-name: k8s.io/client-go dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Description:
Kubernetes processor can be used to enrich not only spans but metrics with k8s metadata. In order to find a source of the metrics we take a different approach then for spans. The processor looks at "host.hostname" resource attribute which is set by prometheus receiver and some metrics instrumentation libraries to an originated IP address. That IP address is used by k8s processor to lookup for a k8s pod and fetch its metadata.
The processor uses the old-style metrics data model since the new metric model is not available in contrib yet. But it must be easy to switch over later once we wave it the new metrics model support.
Testing:
Documentation:
Added details about metrics to existing docs.