Skip to content

Conversation

guswynn
Copy link
Contributor

@guswynn guswynn commented Jan 9, 2024

Alright, this is a doozy:

The bottom commit is primarily bringing in tokio-rs/tracing-opentelemetry#86, through:

the bottom commit also updates various related crates to the new version that tracing-opentelemetry uses.

The second commit attempts to resolve conflicts for ordered-float;

The 3rd commit certifies the new deps with cargo-vet (it adds an exemption for the new serde-value version from our fork that just bumps its ordered-float dep)

Motivation

  • This PR fixes a recognized bug.

Fixes https://github.com/MaterializeInc/database-issues/issues/4358

Checklist

  • This PR has adequate test coverage / QA involvement has been duly considered.
  • This PR has an associated up-to-date design doc, is a design doc (template), or is sufficiently small to not require a design.
  • If this PR evolves an existing $T ⇔ Proto$T mapping (possibly in a backwards-incompatible way), then it is tagged with a T-proto label.
  • If this PR will require changes to cloud orchestration or tests, there is a companion cloud PR to account for those changes that is tagged with the release-blocker label (example).
  • This PR includes the following user-facing behavior changes:

@guswynn guswynn requested a review from benesch as a code owner January 9, 2024 20:44
@guswynn guswynn requested review from a team January 9, 2024 20:44
@guswynn guswynn requested a review from a team as a code owner January 9, 2024 20:44
@guswynn guswynn requested a review from ParkMyCar January 9, 2024 20:44
@guswynn guswynn force-pushed the otel-update branch 2 times, most recently from 86f322f to 2277a43 Compare January 9, 2024 20:50
@guswynn guswynn changed the title tracing: update tracing-opentelemetry tracing: update tracing-opentelemetry with panic fix Jan 9, 2024
Copy link
Contributor

@ParkMyCar ParkMyCar left a comment

Choose a reason for hiding this comment

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

  1. Upgrade in MaterializeInc/tracing-opentelemetry@1c87231 looks good
  2. One thought on the on_follows_from API, no idea if the scenario I'm thinking of is possible though!

Otherwise things look good, excited that we finally tracked down this issue!! 💯

@guswynn
Copy link
Contributor Author

guswynn commented Jan 9, 2024

updated with MaterializeInc/tracing-opentelemetry#1

@guswynn
Copy link
Contributor Author

guswynn commented Jan 9, 2024

@ParkMyCar Im gonna merge this so we try it out, but let me think more about: MaterializeInc/tracing-opentelemetry@1e0cf8b#r136643682

@guswynn guswynn merged commit a68822a into MaterializeInc:main Jan 9, 2024
@guswynn guswynn deleted the otel-update branch January 11, 2024 17:03
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.

2 participants