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

[cmd/telemetrygen] Remove WithBlock from telemetrygen metrics/traces #31631

Merged

Conversation

AlexDCraig
Copy link
Contributor

@AlexDCraig AlexDCraig commented Mar 6, 2024

Description:

Metrics and traces in telemetrygen were sent with WithBlock() which was a blocking call waiting on a successful connection to be formed. In the event a connection could not succeed, like a TLS handshake failure, the block would loop indefinitely.

Link to tracking Issue: Resolves #31401

Testing:

  • Run telemetrygen on main pointed at a host that won't resolve TLS and observe that the command loops indefinitely
  • Run telemetrygen on this commit pointed at that same host and observe it exit after failing

Documentation:

@AlexDCraig AlexDCraig requested review from mx-psi, codeboten and a team as code owners March 6, 2024 23:38
@github-actions github-actions bot added the cmd/telemetrygen telemetrygen command label Mar 6, 2024
@AlexDCraig AlexDCraig force-pushed the dev/fix-tls-handshake-telemetrygen branch from 2c1792a to daa35ed Compare March 6, 2024 23:49
Copy link
Member

@mx-psi mx-psi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@crobert-1
Copy link
Member

Nit: Can you update the PR description to say:

Link to tracking Issue:  Resolves #31401

Having the word Resolves (or Fixes) allows our Github automation to automatically close the issue, reducing the chance we forget to do it ourselves 👍

Copy link
Member

@crobert-1 crobert-1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing!

@dmitryax dmitryax merged commit 039b606 into open-telemetry:main Mar 8, 2024
142 checks passed
@github-actions github-actions bot added this to the next release milestone Mar 8, 2024
@AlexDCraig AlexDCraig deleted the dev/fix-tls-handshake-telemetrygen branch March 8, 2024 16:35
DougManton pushed a commit to DougManton/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
…pen-telemetry#31631)

**Description:**

Metrics and traces in telemetrygen were sent with WithBlock() which was
a blocking call waiting on a successful connection to be formed. In the
event a connection could not succeed, like a TLS handshake failure, the
block would loop indefinitely.

**Link to tracking Issue:** Resolves
open-telemetry#31401

**Testing:**

- Run telemetrygen on main pointed at a host that won't resolve TLS and
observe that the command loops indefinitely
- Run telemetrygen on this commit pointed at that same host and observe
it exit after failing

**Documentation:** <Describe the documentation added.>
XinRanZhAWS pushed a commit to XinRanZhAWS/opentelemetry-collector-contrib that referenced this pull request Mar 13, 2024
…pen-telemetry#31631)

**Description:**

Metrics and traces in telemetrygen were sent with WithBlock() which was
a blocking call waiting on a successful connection to be formed. In the
event a connection could not succeed, like a TLS handshake failure, the
block would loop indefinitely.

**Link to tracking Issue:** Resolves
open-telemetry#31401

**Testing:**

- Run telemetrygen on main pointed at a host that won't resolve TLS and
observe that the command loops indefinitely
- Run telemetrygen on this commit pointed at that same host and observe
it exit after failing

**Documentation:** <Describe the documentation added.>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd/telemetrygen telemetrygen command
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[cmd/telemetrygen] Duration flag is not respected when tls handshake fails
5 participants