-
Notifications
You must be signed in to change notification settings - Fork 41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix otel-collector logging repeat error #748
Fix otel-collector logging repeat error #748
Conversation
92edac5
to
1fa4685
Compare
b6b15f2
to
cc7f109
Compare
21226a4
to
2940f66
Compare
3645a97
to
5154183
Compare
b98365a
to
82f3307
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
close to LGTM, but a few more comments
1a4932a
to
147ed2c
Compare
147ed2c
to
9e3d346
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
couple nits but mostly LGTM
c53b20e
to
c907f4b
Compare
/retest-required |
e2e/testcases/otel_collector_test.go
Outdated
nt.T.Log("Restart otel-collector pod to reset the ConfigMap and log") | ||
nomostest.DeletePodByLabel(nt, "app", ocmetrics.OpenTelemetry, false) | ||
if err := nt.Watcher.WatchForCurrentStatus(kinds.Deployment(), ocmetrics.OtelCollectorName, ocmetrics.MonitoringNamespace); err != nil { | ||
nt.T.Log(fmt.Sprintf("otel-collector pod failed to come up after a restart: %v", err)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Final nit: Replace nt.T.Log(fmt.Sprintf(...
with nt.T.Errorf(...
. Ditto below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any new changes, you may have forgotten to push :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to amend the commit, now good
The pod log shows repeat log of duplicate timeseries error that takes up logging quota. Although the metris are correctly being sent, the error message can be confusing to users. The removal of `type` tag is causing one timeseries being exported multiple times in batch with and without the tag. This change uses aggregation in metricstransfrom processor to eliminate the `type` tag from Monarch pipeline. Format follows otel-collector-contrib 0.54.0. Tested locally e2e. b/290678742
c907f4b
to
9249170
Compare
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sdowell The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/unhold |
507f59a
into
GoogleContainerTools:main
This merged during code freeze... is it needed for v1.16.0? |
It is no longer targeting for 1.16. It was merged after the projected preview date, in theory the release candidate is confirmed by then. |
The pod log shows repeat log of duplicate timeseries error that takes up logging quota. Although the metris are correctly being sent, the error message can be confusing to users. The removal of `type` tag is causing one timeseries being exported multiple times in batch with and without the tag. This change uses aggregation in metricstransfrom processor to eliminate the `type` tag from Monarch pipeline. Format follows otel-collector-contrib 0.54.0. Tested locally e2e. b/290678742
The pod log shows repeat log of duplicate timeseries error that takes up logging quota. Although the metris are correctly being sent, the error message can be confusing to users. The removal of `type` tag is causing one timeseries being exported multiple times in batch with and without the tag. This change uses aggregation in metricstransfrom processor to eliminate the `type` tag from Monarch pipeline. Format follows otel-collector-contrib 0.54.0. Tested locally e2e. b/290678742
The otel-collector pod log shows repeat entries of duplicate timeseries error that takes up logging quota. Although the metris are correctly being sent, the error message can be confusing to users.
The removal of
type
tag is causing one timeseries being processed multiple times inbatch
, both with and without the tag, which causes the above error.This change uses aggregation in metricstransfrom processor to eliminate the
type
tag from Monarch pipeline. Format follows otel-collector-contrib 0.54.0.b/290678742