Skip to content

chore: bump otel to 128#3843

Merged
korniltsev merged 21 commits intomainfrom
korniltsev/otel128
Jun 20, 2025
Merged

chore: bump otel to 128#3843
korniltsev merged 21 commits intomainfrom
korniltsev/otel128

Conversation

@korniltsev
Copy link
Copy Markdown
Contributor

@korniltsev korniltsev commented Jun 16, 2025

PR Description

Continuation of #3841

Bump otel dependencies from 127 to 128

Which issue(s) this PR fixes

The new otel ebpf profiler PR #2920 requires upgrading to 128.

This PR is only upgrading the dependency and fixes compilation to unblock the above profiling PR. The proper upgrade should be handled here #3844

Notes to the Reviewer

PR Checklist

  • CHANGELOG.md updated
  • Documentation added
  • Tests updated
  • Config converters updated

@korniltsev korniltsev marked this pull request as ready for review June 18, 2025 11:54
@korniltsev korniltsev requested a review from a team as a code owner June 18, 2025 11:54
@clayton-cornell
Copy link
Copy Markdown
Contributor

Is this a version change that should also be reflected in the main _index.md and _index.md.t files in the documentation?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jun 19, 2025

💻 Deploy preview deleted.

@korniltsev
Copy link
Copy Markdown
Contributor Author

Is this a version change that should also be reflected in the main _index.md and _index.md.t files in the documentation?

Updated the markdowns.

This PR is only upgrading the dependency and fixes compilation to unblock the profiling PR. The proper upgrade should be handled here #3844 . I don't have capacity to do the proper upgrade.

Comment thread CHANGELOG.md Outdated
EndpointParams: args.EndpointParams,
Scopes: args.Scopes,
TLSSetting: *args.TLSSetting.Convert(),
TLS: *args.TLSSetting.Convert(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you rename args.TLSSetting also to args.TLS?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

renamed

Comment on lines -369 to -371
{
name: "push_config and remote_write",
cfg: `
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why removing this test case?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

it was fragile and incorrect and was testing for wrong thing.

It was supposed to test for an error when push_config and remote_write are configured together.

But in fact (before upgrade to v128) it was receiving a completely different error "empty config for Jaeger receiver".

And the test did check for the presence of error, not the value/type of the error and the test was incorrectly green.

Before v128 the nil map was unmarshaled an empty map and failed validation leading to an error and now after v128 it is unmarshaled as default configuration with no error.

Anyway the tests should be removed because push_config was removed long ago

@korniltsev korniltsev merged commit f469833 into main Jun 20, 2025
41 of 42 checks passed
@korniltsev korniltsev deleted the korniltsev/otel128 branch June 20, 2025 09:19
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Jul 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants