Skip to content

Conversation

yiyuan-he
Copy link
Contributor

@yiyuan-he yiyuan-he commented Jun 16, 2025

What does this pull request do?

This reverts commit a538775 to re-apply the OTel dependency update in our ADOT SDK since the root cause of the failing contract tests was fixed: #398.

We also bump the OTel dependency version to sync with our ADOT SDK bump. This solves the issue of our contract tests not catching version bump issues in the PR build.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@yiyuan-he yiyuan-he requested a review from a team as a code owner June 16, 2025 17:38
@yiyuan-he yiyuan-he force-pushed the revert-otel-bump-revert branch from f69782a to f6931f9 Compare June 16, 2025 18:46
@yiyuan-he yiyuan-he force-pushed the revert-otel-bump-revert branch from de23d78 to 0977f8a Compare June 16, 2025 21:54
@yiyuan-he yiyuan-he force-pushed the revert-otel-bump-revert branch from 0977f8a to 4716d6b Compare June 16, 2025 22:09
@yiyuan-he yiyuan-he changed the title Reapply "Bump OTel Dependency Versions to 1.33.0/0.54b0" (#397) Bump OTel Dependency Versions to 1.33.0/0.54b0 Jun 17, 2025
@yiyuan-he yiyuan-he changed the title Bump OTel Dependency Versions to 1.33.0/0.54b0 Reapply Bump OTel Dependency Versions to 1.33.0/0.54b0 Jun 17, 2025
Copy link
Contributor

@thpierce thpierce left a comment

Choose a reason for hiding this comment

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

  1. Lets update to 1.33.1, not 1.33.0.
  2. We override a good chunk of of upstream code in aws_opentelemetry_configurator, here. I worry that the upstream may have changed in a subtle way. I think we should do an upstream comparison as a sanity check. Basically just compare each method to the upstream @1.33.1 to see what the diff is.

@yiyuan-he yiyuan-he changed the title Reapply Bump OTel Dependency Versions to 1.33.0/0.54b0 Reapply Bump OTel Dependency Versions to 1.33.1/0.54b1 Jun 18, 2025
@yiyuan-he yiyuan-he force-pushed the revert-otel-bump-revert branch 2 times, most recently from 566c3c9 to 4c02feb Compare June 18, 2025 04:54
@yiyuan-he yiyuan-he force-pushed the revert-otel-bump-revert branch from 4c02feb to d176d50 Compare June 18, 2025 05:00
@yiyuan-he yiyuan-he changed the title Reapply Bump OTel Dependency Versions to 1.33.1/0.54b1 Bump OTel Dependency Versions to 1.33.1/0.54b1 Jun 18, 2025
@yiyuan-he
Copy link
Contributor Author

  1. Lets update to 1.33.1, not 1.33.0.
  2. We override a good chunk of of upstream code in aws_opentelemetry_configurator, here. I worry that the upstream may have changed in a subtle way. I think we should do an upstream comparison as a sanity check. Basically just compare each method to the upstream @1.33.1 to see what the diff is.
  1. Updated version bump to 1.33.1 instead of 1.33.0
  2. Synced offline with @srprash - confirmed any divergence we have with the upstream code in aws_opentelemetry_configurator is non-blocking.

@yiyuan-he
Copy link
Contributor Author

FYI synced offline with @thp - initial round of comments have been addressed. Will rely on @mxiamxia and @srprash as final reviewers/approvals.

@yiyuan-he yiyuan-he force-pushed the revert-otel-bump-revert branch from d307d9e to deb4baa Compare June 18, 2025 19:13
mxiamxia
mxiamxia previously approved these changes Jun 18, 2025
@yiyuan-he yiyuan-he force-pushed the revert-otel-bump-revert branch from 53fd0f5 to 22ad013 Compare June 19, 2025 04:16
@yiyuan-he yiyuan-he force-pushed the revert-otel-bump-revert branch from 8da7c01 to babfd66 Compare June 19, 2025 20:23
@yiyuan-he yiyuan-he force-pushed the revert-otel-bump-revert branch from babfd66 to 80c3296 Compare June 19, 2025 20:34
@yiyuan-he yiyuan-he force-pushed the revert-otel-bump-revert branch from e486611 to 5f18b40 Compare June 19, 2025 20:46
@yiyuan-he yiyuan-he merged commit c42d1b6 into aws-observability:main Jun 19, 2025
13 checks passed
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.

4 participants