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.

Sign up for GitHub

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

Jump to bottom

opentelemetry: Update otel to 0.18.0 #2303

Merged
merged 4 commits into from Sep 15, 2022
Merged

opentelemetry: Update otel to 0.18.0 #2303

merged 4 commits into from Sep 15, 2022

Conversation

jtescher
Copy link
Collaborator

Motivation

Support the latest OpenTelemetry specification.

Solution

Update opentelemetry to the latest 0.18.x release. Breaking changes in the metrics spec have removed value recorders and added histograms so the metrics layer's value. prefix has been changed to histogram. and behaves accordingly. Additionally the PushController configuration for the metrics layer has been simplified to accept a BasicController that can act in either push or pull modes. Finally trace sampling in the sdk's PreSampledTracer impl has been updated to match the sampling logic in open-telemetry/opentelemetry-rust#839.

@jtescher
opentelemetry: Update otel to 0.18.0
29cbb7d 
## Motivation

Support the latest OpenTelemetry specification.

## Solution

Update `opentelemetry` to the latest `0.18.x` release. Breaking changes
in the metrics spec have removed value recorders and added histograms so
the metrics layer's `value.` prefix has been changed to `histogram.` and
behaves accordingly. Additionally the `PushController` configuration for
the metrics layer has been simplified to accept a `BasicController` that
can act in either push or pull modes. Finally trace sampling in the
sdk's `PreSampledTracer` impl has been updated to match the sampling
logic in open-telemetry/opentelemetry-rust#839.
@jtescher jtescher added the crate/opentelemetry Related to the `tracing-opentelemetry` crate. label Sep 13, 2022
@jtescher jtescher requested a review from a team as a code owner September 13, 2022 03:36
This was referenced Sep 14, 2022
Closed
Closed
@hawkw
Copy link
Member

hawkw commented Sep 15, 2022

Build faiilure with -Zminimal-versions (https://github.com/tokio-rs/tracing/actions/runs/3055323497/jobs/4928284929) is unrelated, I'll fix that.

@hawkw
Copy link
Member

hawkw commented Sep 15, 2022 •
edited

Build faiilure with -Zminimal-versions (https://github.com/tokio-rs/tracing/actions/runs/3055323497/jobs/4928284929) is unrelated, I'll fix that.

Actually, I don't think this is unrelated --- the failing async-trait version is pulled as a dependency of opentelemetry. However, I'm unsure why this is happening, as we depend on a newer async-trait version specifically to prevent this:

tracing/tracing-opentelemetry/Cargo.toml

Lines 35 to 36 in 84633ea

# Fix minimal-versions; opentelemetry specifies async-trait = "0.1" which breaks
async-trait = "0.1.20"

so I'm unsure what's wrong here...

Never mind, looks like this branch actually removes those. Putting them back should fix this: #2303 (review)

hawkw
hawkw requested changes Sep 15, 2022
View reviewed changes
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.

It looks like the minimal versions build failure is caused by the removal of explicit dependencies on async-trait in tracing-opentelemetry's Cargo.toml.
I think if we put those back, the -Zminimal-versions CI build will pass again and we can move forwards with this PR.

@jtescher is there a reason the async-trait dependencies were removed? If we needed to get rid of them for some reason, that will complicate fixing the minimal-versions build...

tracing-opentelemetry/Cargo.toml Outdated Show resolved Hide resolved
tracing-opentelemetry/Cargo.toml Show resolved Hide resolved
@jtescher
Copy link
Collaborator Author

jtescher commented Sep 15, 2022 •
edited

@hawkw ah fixed, the async-trait usage in this crate had been removed, but not the dependencies. Looks like these are passing now besides the nightly compiler bug.

hawkw
hawkw approved these changes Sep 15, 2022
View reviewed changes
Copy link
Member

@hawkw hawkw left a comment •
edited

Choose a reason for hiding this comment

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

okay, great, now that the build passes, this LGTM!

I think we can safely ignore the nightly build failure, though I'm going to see if there's an existing bug report for this...

Edit: ah, looks like that's rust-lang/rust#101844

@hawkw hawkw merged commit 982526f into master Sep 15, 2022
@hawkw hawkw deleted the jtescher/update-otel branch September 15, 2022 23:54
@purkhusid
Copy link

Is it possible to push a new release of tracing-opentelemetry? I think this PR should fix #2306

jtescher added a commit that referenced this pull request Sep 16, 2022
@jtescher
opentelemetry: update otel to 0.18.0 (#2303)
ff4ca28 
## Motivation

Support the latest OpenTelemetry specification.

## Solution

Update `opentelemetry` to the latest `0.18.x` release. Breaking changes
in the metrics spec have removed value recorders and added histograms so
the metrics layer's `value.` prefix has been changed to `histogram.` and
behaves accordingly. Additionally the `PushController` configuration for
the metrics layer has been simplified to accept a `BasicController` that
can act in either push or pull modes. Finally trace sampling in the
sdk's `PreSampledTracer` impl has been updated to match the sampling
logic in open-telemetry/opentelemetry-rust#839.

* Update MSRV to 1.56
* Update examples
* Fix async-trait dep
# Conflicts:
#	.github/workflows/CI.yml
#	tracing-opentelemetry/Cargo.toml
#	tracing-opentelemetry/src/layer.rs
#	tracing-opentelemetry/src/metrics.rs
#	tracing-opentelemetry/tests/metrics_publishing.rs
@jtescher jtescher mentioned this pull request Sep 16, 2022
Merged
hawkw added a commit that referenced this pull request Sep 16, 2022
@jtescher @hawkw
opentelemetry: update otel to 0.18.0 (#2303)
10a4b13 
## Motivation

Support the latest OpenTelemetry specification.

## Solution

Update `opentelemetry` to the latest `0.18.x` release. Breaking changes
in the metrics spec have removed value recorders and added histograms so
the metrics layer's `value.` prefix has been changed to `histogram.` and
behaves accordingly. Additionally the `PushController` configuration for
the metrics layer has been simplified to accept a `BasicController` that
can act in either push or pull modes. Finally trace sampling in the
sdk's `PreSampledTracer` impl has been updated to match the sampling
logic in open-telemetry/opentelemetry-rust#839.

* Update MSRV to 1.56
* Update examples
* Fix async-trait dep
* Update msrv action

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
@jtescher jtescher mentioned this pull request Sep 18, 2022
Merged
hawkw added a commit that referenced this pull request Sep 19, 2022
@jtescher @hawkw
opentelemetry: update otel to 0.18.0 (#2303)
bd56070 
## Motivation

Support the latest OpenTelemetry specification.

## Solution

Update `opentelemetry` to the latest `0.18.x` release. Breaking changes
in the metrics spec have removed value recorders and added histograms so
the metrics layer's `value.` prefix has been changed to `histogram.` and
behaves accordingly. Additionally the `PushController` configuration for
the metrics layer has been simplified to accept a `BasicController` that
can act in either push or pull modes. Finally trace sampling in the
sdk's `PreSampledTracer` impl has been updated to match the sampling
logic in open-telemetry/opentelemetry-rust#839.

* Update MSRV to 1.56
* Update examples
* Fix async-trait dep
* Update msrv action

Co-authored-by: Eliza Weisman <eliza@buoyant.io>
hawkw pushed a commit that referenced this pull request Sep 19, 2022
@jtescher
opentelemetry: prepare for v0.18.0 release (#2312)
a4f2ee2 
### Breaking Changes

- Upgrade to `v0.18.0` of `opentelemetry` (#2303)
  For list of breaking changes in OpenTelemetry, see the
  [v0.18.0 changelog](https://github.com/open-telemetry/opentelemetry-rust/blob/main/opentelemetry/CHANGELOG.md#v0180).

### Fixed

- `on_event` respects event's explicit parent (#2296)
@pabrahamsson pabrahamsson mentioned this pull request Feb 11, 2023
Merged
@notheotherben notheotherben mentioned this pull request Aug 14, 2023
Closed
@davidbarsky davidbarsky mentioned this pull request Sep 26, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hawkw hawkw hawkw approved these changes

@davidbarsky davidbarsky Awaiting requested review from davidbarsky davidbarsky is a code owner

Assignees
No one assigned
Labels
crate/opentelemetry Related to the `tracing-opentelemetry` crate.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jtescher @hawkw @purkhusid