Skip to content
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: avoid redundant logs on failures to export metrics #20519

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

jjti
Copy link
Contributor

@jjti jjti commented Feb 7, 2024

Description

I'm seeing redundant log lines on failures to push metrics to HCP:

2024-02-07T14:22:37.358Z [ERROR] agent.hcp.hcp_telemetry_client: request failed: error="Post "https://consul-telemetry.hcp.dev/otlp/v1/metrics\": http2: Transport: cannot retry err [http2: Transport received Server's graceful shutdown GOAWAY] after Request.Body was written; define Request.GetBody to avoid this error" method=POST url=https://consul-telemetry.hcp.dev/otlp/v1/metrics

2024/02/07 14:22:37 failed to export metrics: failed to post metrics: POST https://consul-telemetry.hcp.dev/otlp/v1/metrics giving up after 1 attempt(s): Post "https://consul-telemetry.hcp.dev/otlp/v1/metrics": http2: Transport: cannot retry err [http2: Transport received Server's graceful shutdown GOAWAY] after Request.Body was written; define Request.GetBody to avoid this error

We log in two places:

The fix here is to pass a NullLogger to the http client so:

An alternative approach is to combine this PR with the other logging-related PR: #20514. And:

  • set the otel global logger to a no-op logger that does nothing
  • also disable logging in the go-retryablehttp client (NullLogger as in this PR)
  • only log in ExportMetrics

Testing & Reproduction steps

Links

PR Checklist

  • updated test coverage
  • external facing docs updated
  • appropriate backport labels added
  • not a security concern

@jjti jjti added backport/1.16 This release series is no longer active on CE. Use backport/ent/1.16. backport/1.17 This release series is no longer active on CE. Use backport/ent/1.17. backport/1.18 labels Feb 7, 2024
@jjti jjti requested review from loshz and Achooo February 7, 2024 19:31
@jjti jjti added the pr/no-changelog PR does not need a corresponding .changelog entry label Feb 7, 2024
@jjti jjti merged commit c790740 into main Feb 8, 2024
95 of 96 checks passed
@jjti jjti deleted the jjti/fix-hcp-client-logs branch February 8, 2024 16:00
@hc-github-team-consul-core
Copy link
Collaborator

@hc-github-team-consul-core
Copy link
Collaborator

@hc-github-team-consul-core
Copy link
Collaborator

@jjti, a backport is missing for this PR [20519] for versions [1.16,1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

3 similar comments
@hc-github-team-consul-core
Copy link
Collaborator

@jjti, a backport is missing for this PR [20519] for versions [1.16,1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@jjti, a backport is missing for this PR [20519] for versions [1.16,1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@hc-github-team-consul-core
Copy link
Collaborator

@jjti, a backport is missing for this PR [20519] for versions [1.16,1.17] please perform the backport manually and add the following snippet to your backport PR description:

<details>
	<summary> Overview of commits </summary>
		- <<backport commit 1>>
		- <<backport commit 2>>
		...
</details>

@loshz loshz removed backport/1.16 This release series is no longer active on CE. Use backport/ent/1.16. backport/1.17 This release series is no longer active on CE. Use backport/ent/1.17. labels Feb 12, 2024
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr/no-changelog PR does not need a corresponding .changelog entry pr/no-metrics-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants