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

Add support for OTLP tracing reporting #34

Merged
merged 8 commits into from
May 24, 2024
Merged

Add support for OTLP tracing reporting #34

merged 8 commits into from
May 24, 2024

Conversation

inikulin
Copy link
Collaborator

It's recommended to review the commits in the PR separately

@inikulin inikulin force-pushed the tracing-exporters branch 2 times, most recently from f7307e8 to c37827b Compare March 11, 2024 18:02
#[cfg(feature = "telemetry-server")]
server_fut: Option<TelemetryServerFuture>,

tele_futures: FuturesUnordered<BoxFuture<'static, BootstrapResult<()>>>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this is always going to have one future, why is this different from server_fut

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, in the upcoming PRs it will be also used to drive exporters.

This allows to get rid of 2 separate functions for telemetry initialisation and allows to drive async functionality other than just the telemetry server.
- Use async reporting for traces by using rustracing fork
- Scaffold gRPC reporter
- Fix defaults for settings - make serde(default) attributes unconditional, otherwise they were not parsed by our macro
@inikulin inikulin force-pushed the tracing-exporters branch 3 times, most recently from 9371eba to 28ac05a Compare May 9, 2024 18:41
@inikulin inikulin changed the title [DO NOT MERGE YET] Some v4.0.0 API changes in preparation for OTLP support Add support for OTLP tracing reporting May 9, 2024
@inikulin inikulin force-pushed the tracing-exporters branch 4 times, most recently from 81b3e54 to 767ae02 Compare May 10, 2024 11:59
if ready_res.is_empty() {
Poll::Pending
} else {
Poll::Ready(ready_res.into_iter().collect())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is completing after the first polled future finishes the intended behavior here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes. There are 2 cases where either of those complete:

  1. TelemetryServer received a graceful shutdown signal. Once it finishes we wrap up all operations, including telemetry reporting
  2. Any of those error - one of the components of the telemetry system is not functioning anymore, we need to terminate.

@inikulin inikulin merged commit 5642ad7 into main May 24, 2024
17 checks passed
@inikulin inikulin deleted the tracing-exporters branch May 24, 2024 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants