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

opentelemetry: Update otel to 0.17.0 #1853

Merged
merged 7 commits into from
Jan 27, 2022
Merged

opentelemetry: Update otel to 0.17.0 #1853

merged 7 commits into from
Jan 27, 2022

Conversation

jtescher
Copy link
Collaborator

@jtescher jtescher commented Jan 22, 2022

Motivation

Support the latest OpenTelemetry specification. Resolves #1857.

Solution

Update opentelemetry to the latest 0.17.x release. Breaking changes upstream in the tracking of parent contexts in otel's SpanBuilder have necessitated a new OtelData struct to continue pairing tracing spans with their associated otel Context.

## Motivation

Support the latest OpenTelemetry specification.

## Solution

Update `opentelemetry` to the latest `0.17.x` release. Breaking changes
upstream in the tracking of parent contexts in otel's `SpanBuilder` have
necessitated a new `OtelData` struct to continue pairing tracing spans
with their associated otel `Context`.
@jtescher jtescher added the crate/opentelemetry Related to the `tracing-opentelemetry` crate. label Jan 22, 2022
@jtescher jtescher requested review from davidbarsky, hawkw and a team as code owners January 22, 2022 17:55
@davidbarsky
Copy link
Member

For what it’s worth, tracing-opentelemetry can abide by a higher MSRV, especially if the opentelemetry has a higher one. tracing-appender already does because the time crate has a higher MSRV.

@hawkw
Copy link
Member

hawkw commented Jan 24, 2022

Looks like CI is failing because opentelemetry increased its MSRV: https:/tokio-rs/tracing/runs/4908254944?check_suite_focus=true#step:4:151

Since the opentelemetry dependency is (of course) required for tracing-opentelemetry, we should just increase the crate's MSRV to whatever opentelemetry requires. We can fix the CI build by adding a separate check job for tracing-opentelemetry on its new MSRV, the way we do for tracing-appender:

check-msrv-appender:
# Run `cargo check` on our minimum supported Rust version (1.51.0).
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@main
- uses: actions-rs/toolchain@v1
with:
toolchain: 1.53.0
profile: minimal
override: true
- name: Check
uses: actions-rs/cargo@v1
with:
command: check
args: --lib=tracing-appender

We'll also have to add tracing-opentelemetry to the list of crates excluded from the main MSRV check job, here:

args: --all --exclude=tracing-appender

In order to increase the crate's MSRV, we'll have to update the rust-version field in Cargo.toml:

rust-version = "1.42.0"
as well as the MSRV documented in the README and lib.rs documentation.

@jtescher, can I get you to update the MSRV so that we can get CI to pass? Then, I think this should probably be good to merge. Thanks!

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

overall, this looks good to me, but we'll need to fix the MSRV issues I mentioned before we can merge this. I commented on a couple other small things.

tracing-opentelemetry/src/subscriber.rs Outdated Show resolved Hide resolved

/// Per-span OpenTelemetry data tracked by this crate.
#[derive(Debug, Clone)]
pub struct OtelData {
Copy link
Member

Choose a reason for hiding this comment

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

Am I correct that this needs to be public because it's used in the PreSampledTracer trait, but it's totally opaque to user code? Is there any reason user code might need to access or mutate this type's fields?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point, fields need to be pub for althernate sdk implementations. Will fix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added quick comment as well to clarify purpose a bit. Should be opaque to everyone who is not trying to implement PreSampledTracer directly.

@jtescher
Copy link
Collaborator Author

@hawkw ok great all passing now ✅

Copy link
Member

@hawkw hawkw left a comment

Choose a reason for hiding this comment

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

lgtm

Tracing is built against the latest stable release. The minimum supported
version is 1.42. The current Tracing version is not guaranteed to build on Rust
versions earlier than the minimum supported version.
Tracing Opentelemetry is built against the latest stable release. The minimum
Copy link
Member

Choose a reason for hiding this comment

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

nit: should this be

Suggested change
Tracing Opentelemetry is built against the latest stable release. The minimum
Tracing OpenTelemetry is built against the latest stable release. The minimum

@@ -111,6 +111,8 @@ pub use subscriber::{subscriber, OpenTelemetrySubscriber};
pub use tracer::PreSampledTracer;

/// Per-span OpenTelemetry data tracked by this crate.
///
/// Useful for implementing [PreSampledTracer] in alternate otel SDKs.
Copy link
Member

Choose a reason for hiding this comment

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

nit

Suggested change
/// Useful for implementing [PreSampledTracer] in alternate otel SDKs.
/// Useful for implementing [`PreSampledTracer`] in alternate otel SDKs.

@hawkw hawkw enabled auto-merge (squash) January 27, 2022 19:03
@hawkw hawkw merged commit e005548 into master Jan 27, 2022
@hawkw hawkw deleted the jtescher/update-otel branch January 27, 2022 19:13
@caizixian
Copy link

I'm just wondering whether a new release can be made so that downstream projects can update their dependencies accordingly.

jtescher added a commit that referenced this pull request Jan 30, 2022
## Motivation

Support the latest OpenTelemetry specification.

## Solution

Update `opentelemetry` to the latest `0.17.x` release. Breaking changes
upstream in the tracking of parent contexts in otel's `SpanBuilder` have
necessitated a new `OtelData` struct to continue pairing tracing spans
with their associated otel `Context`.
# Conflicts:
#	.github/workflows/check_msrv.yml
#	tracing-opentelemetry/Cargo.toml
#	tracing-opentelemetry/benches/trace.rs
#	tracing-opentelemetry/src/layer.rs
#	tracing-opentelemetry/src/span_ext.rs
#	tracing-opentelemetry/tests/trace_state_propagation.rs
jtescher added a commit that referenced this pull request Jan 30, 2022
## Motivation

Support the latest OpenTelemetry specification.

## Solution

Update `opentelemetry` to the latest `0.17.x` release. Breaking changes
upstream in the tracking of parent contexts in otel's `SpanBuilder` have
necessitated a new `OtelData` struct to continue pairing tracing spans
with their associated otel `Context`.
# Conflicts:
#	.github/workflows/check_msrv.yml
#	tracing-opentelemetry/Cargo.toml
#	tracing-opentelemetry/benches/trace.rs
#	tracing-opentelemetry/src/layer.rs
#	tracing-opentelemetry/src/span_ext.rs
#	tracing-opentelemetry/tests/trace_state_propagation.rs
hawkw pushed a commit that referenced this pull request Feb 3, 2022
## Motivation

Support the latest OpenTelemetry specification.

## Solution

Update `opentelemetry` to the latest `0.17.x` release. Breaking changes
upstream in the tracking of parent contexts in otel's `SpanBuilder` have
necessitated a new `OtelData` struct to continue pairing tracing spans
with their associated otel `Context`.
# Conflicts:
#	.github/workflows/check_msrv.yml
#	tracing-opentelemetry/Cargo.toml
#	tracing-opentelemetry/benches/trace.rs
#	tracing-opentelemetry/src/layer.rs
#	tracing-opentelemetry/src/span_ext.rs
#	tracing-opentelemetry/tests/trace_state_propagation.rs
hawkw pushed a commit that referenced this pull request Feb 3, 2022
Breaking changes in this release:

- Upgrade to `v0.17.0` of `opentelemetry` (#1853)
  For list of breaking changes in OpenTelemetry, see the [v0.17.0 changelog](https:/open-telemetry/opentelemetry-rust/blob/main/opentelemetry/CHANGELOG.md#v0170).
hawkw pushed a commit that referenced this pull request Feb 3, 2022
Breaking changes in this release:

- Upgrade to `v0.17.0` of `opentelemetry` (#1853)
  For list of breaking changes in OpenTelemetry, see the [v0.17.0 changelog](https:/open-telemetry/opentelemetry-rust/blob/main/opentelemetry/CHANGELOG.md#v0170).
hawkw pushed a commit that referenced this pull request Feb 3, 2022
Breaking changes in this release:

- Upgrade to `v0.17.0` of `opentelemetry` (#1853)
  For list of breaking changes in OpenTelemetry, see the [v0.17.0 changelog](https:/open-telemetry/opentelemetry-rust/blob/main/opentelemetry/CHANGELOG.md#v0170).
hawkw pushed a commit that referenced this pull request Feb 3, 2022
Breaking changes in this release:

- Upgrade to `v0.17.0` of `opentelemetry` (#1853)
  For list of breaking changes in OpenTelemetry, see the [v0.17.0 changelog](https:/open-telemetry/opentelemetry-rust/blob/main/opentelemetry/CHANGELOG.md#v0170).
hawkw pushed a commit that referenced this pull request Feb 3, 2022
Breaking changes in this release:

- Upgrade to `v0.17.0` of `opentelemetry` (#1853)
  For list of breaking changes in OpenTelemetry, see the [v0.17.0 changelog](https:/open-telemetry/opentelemetry-rust/blob/main/opentelemetry/CHANGELOG.md#v0170).
hawkw pushed a commit that referenced this pull request Feb 4, 2022
Breaking changes in this release:

- Upgrade to `v0.17.0` of `opentelemetry` (#1853)
  For list of breaking changes in OpenTelemetry, see the [v0.17.0 changelog](https:/open-telemetry/opentelemetry-rust/blob/main/opentelemetry/CHANGELOG.md#v0170).
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
## Motivation

Support the latest OpenTelemetry specification.

## Solution

Update `opentelemetry` to the latest `0.17.x` release. Breaking changes
upstream in the tracking of parent contexts in otel's `SpanBuilder` have
necessitated a new `OtelData` struct to continue pairing tracing spans
with their associated otel `Context`.
# Conflicts:
#	.github/workflows/check_msrv.yml
#	tracing-opentelemetry/Cargo.toml
#	tracing-opentelemetry/benches/trace.rs
#	tracing-opentelemetry/src/layer.rs
#	tracing-opentelemetry/src/span_ext.rs
#	tracing-opentelemetry/tests/trace_state_propagation.rs
kaffarell pushed a commit to kaffarell/tracing that referenced this pull request May 22, 2024
Breaking changes in this release:

- Upgrade to `v0.17.0` of `opentelemetry` (tokio-rs#1853)
  For list of breaking changes in OpenTelemetry, see the [v0.17.0 changelog](https:/open-telemetry/opentelemetry-rust/blob/main/opentelemetry/CHANGELOG.md#v0170).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/opentelemetry Related to the `tracing-opentelemetry` crate.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opentelemetry 0.17 support
4 participants