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

Different uses of timeout on exporters #4497

Open
mx-psi opened this issue Dec 1, 2021 · 4 comments
Open

Different uses of timeout on exporters #4497

mx-psi opened this issue Dec 1, 2021 · 4 comments
Labels
area:exporter enhancement New feature or request

Comments

@mx-psi
Copy link
Member

mx-psi commented Dec 1, 2021

The exporterhelper includes a timeout mechanism, which is enabled by default (see #4455 for more on that), and sets a timeout for the whole Consume[Metrics/Traces/Logs] function call. Several exporters in contrib however rely on Go's HTTP client timeout instead (1, 2, 3, 4, 5), while still using the same timeout option.

This is an issue to discuss:

  1. If a different option name should be used for these exporters use case,
  2. if we decide that using the same name is okay, should these exporters avoid using exporterhelper.TimeoutSettings and use a custom timeout field instead?

This was originally brought up by @codeboten at open-telemetry/opentelemetry-collector-contrib#6414 (comment). For context, in our exporter we don't rely on the global timeout mechanism because we need to do several network calls on a single Consume call and want to timeout on each specific call instead.

@jpkrohling
Copy link
Member

@MovieStoreGuy, this would be a good entry to add to the doc you are creating.

@MovieStoreGuy
Copy link
Contributor

Yeah, I remember seeing this in the kafka exporter and thought it was strange.

I do think I like the idea of option 1 though, having OperationTimeout and NetworkRequestTimeout would help clarify things as I do think each has a purpose and conflating the two would almost require User's to read and understand the setup code.

@jpkrohling
Copy link
Member

@codeboten, are you able to summarize what was discussed yesterday? I had Zoom problems and missed part of the discussion.

@MovieStoreGuy
Copy link
Contributor

Hey @codeboten ,

Was there more to add to this please?

@github-actions github-actions bot added the Stale label Dec 8, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 11, 2024
@mx-psi mx-psi reopened this Jan 11, 2024
@github-actions github-actions bot removed the Stale label Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:exporter enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants