Conversation
|
💻 Deploy preview deleted (OTel v0.139.0). |
🔍 Dependency ReviewBelow is a review of the Go module dependency changes in this PR, focusing on required code updates to adopt the new versions. Each section includes relevant upstream change references and the concrete code changes (shown as diffs) that this PR already implements or that you should make to maintain behavior.
OpenTelemetry Collector core and contrib 0.134.x → 0.139.xStatus: ❌ Changes Needed This is the main, breaking set of upgrades affecting a wide surface area (telemetry settings, HTTP/GRPC headers types, queue defaults, server keep-alives, component settings, etc.). The PR already includes the required code changes. Key upstream references:
Notable breaking/behavioral changes between 0.134 and 0.139, and code updates required:
Additional doc changes included in this PR:
All of the above changes are necessary to compile and preserve behavior across 0.134 → 0.139.
OpenTelemetry eBPF stack (go.opentelemetry.io/obi and go.opentelemetry.io/ebpf-profiler replacements)Status: ❌ Changes Needed The PR replaces:
These bumps bring API changes in the eBPF profiler/instrumentation APIs:
Code updates in this PR: - cfg.FileObserver = nfs
+ cfg.ExecutableReporter = nfs
- if c.cfg.FileObserver != nil {
- c.cfg.FileObserver.Cleanup()
- }
+ if c.cfg.ExecutableReporter != nil {
+ if nfs, ok := c.cfg.ExecutableReporter.(*irsymcache.Resolver); ok {
+ nfs.Cleanup()
+ }
+ }and in reporter: - ExtraNativeSymbolResolver samples.NativeSymbolResolver
+ ExtraNativeSymbolResolver irsymcache.NativeSymbolResolver
- hostFrame := host.Frame{...}
- irsymcache.SymbolizeNativeFrame(p.cfg.ExtraNativeSymbolResolver, mappingFile.FileName, hostFrame, func(si samples.SourceInfo) {
+ irsymcache.SymbolizeNativeFrame(p.cfg.ExtraNativeSymbolResolver, mappingFile.FileName, fr.AddressOrLineno, mappingFile.FileID, func(si irsymcache.SourceInfo) {Evidence:
No further action needed if you keep the pinned replacements as in this PR. OpenTelemetry Prometheus exporter v0.58.0 → v0.60.0Status: Change:
Code: - return otelprom.New(
- otelprom.WithRegisterer(reg),
- otelprom.WithoutUnits(),
+ return otelprom.New(
+ otelprom.WithRegisterer(reg),
+ otelprom.WithoutUnits(), //nolint:staticcheck // WithoutUnits is deprecated but none of the translation strategies seem correctAction:
github.com/leodido/go-syslog/v4 v4.2.0 → v4.3.0Status: ❌ Changes Needed Behavior change:
Test update required (already done in PR): - require.EqualError(t, results[0].Error, "message too long to parse. was size 8198, max length 8192")
+ require.EqualError(t, results[0].Error, "message length (8198) exceeds maximum length (8192)")Evidence:
github.com/DataDog/go-sqllexer v0.1.6 → v0.1.9Status: ❌ Changes Needed Behavior change:
Test update required (already done in PR): - _, err := ContainsReservedKeywords("SELECT \"foo", ExplainReservedWordDenyList, sqllexer.DBMSMySQL)
+ // TODO - consider if lexer should still error on "foo, see https://github.com/DataDog/go-sqllexer/pull/72
+ _, err := ContainsReservedKeywords("SELECT `foo", ExplainReservedWordDenyList, sqllexer.DBMSMySQL)Evidence:
Kafka libraries
Changes surfaced in this PR (and required with updated clients):
No additional code changes required beyond what the PR includes; ensure compatibility with your brokers if toggling OpenTelemetry collector client/core versions v1.44 → v1.45 (pinned submodules at 0.139.x)Status: ❌ Changes Needed These go hand-in-hand with the 0.139.x contrib changes above. The code adjustments already made in this PR (headers MapList, telemetry settings resource, queue defaults, server keep-alives, etc.) are required for these bumps to compile and preserve behavior. No further changes required. Other dependency bumps (selected)
Notes
If you maintain forks or external modules consuming these APIs, apply the same patterns (MapList for headers, default queue config creation, new server fields, etc.) when upgrading to OpenTelemetry 0.139.x to avoid breakages. |
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades OpenTelemetry Collector to version 0.139.0, bringing various improvements including new cloud provider detectors, Kafka enhancements, S3 exporter features, and telemetry configuration updates.
Key Changes:
- Adds support for 10 new cloud provider resource detectors (Akamai, DigitalOcean, Hetzner, Nova, OracleCloud, Scaleway, UpCloud, Vultr, etc.)
- Enhances Kafka receiver with rack-aware replica selection and partition fetch size controls
- Updates S3 exporter with timezone and base prefix configuration options
- Introduces HTTP keep-alives configuration support across HTTP receivers
- Refactors telemetry settings resource handling with caching mechanism
- Updates queue batch configuration defaults
Reviewed Changes
Copilot reviewed 72 out of 73 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
internal/component/otelcol/util/telemetry_settings.go |
New utility for centralized telemetry resource management with caching |
internal/component/otelcol/processor/resourcedetection/ |
Adds 10 new cloud provider detector configurations |
internal/component/otelcol/receiver/kafka/kafka.go |
Adds rack ID and leader epoch configuration options |
internal/component/otelcol/exporter/kafka/kafka.go |
Adds log partitioning and metadata key options |
internal/component/otelcol/exporter/awss3/awss3.go |
Adds S3 base prefix and timezone configuration |
internal/component/otelcol/config_http.go |
Adds HTTP keep-alives configuration support |
internal/component/otelcol/config_queue.go |
Updates queue batch configuration with new defaults |
internal/converter/internal/otelcolconvert/converter_otlpexporter.go |
Updates header handling for new MapList type |
internal/runtime/componenttest/componenttest.go |
Improves component startup timing with timeout mechanism |
docs/sources/reference/components/otelcol/ |
Updates documentation for new features |
…le reporters (#4827) This change addresses the container ID handling and symbol resolution in the Pyroscope eBPF profiler by: - Replacing FileObserver with ExecutableReporter in the configuration to better reflect its purpose and enable proper cleanup with type assertions - Moving container ID to the proper layer in the event reporting flow by passing it as a parameter to createProfile instead of storing it in ExtraMeta - Simplifying the type system in tests by using upstream types (libpf.FileID and irsymcache.SourceInfo) instead of wrapper types These changes improve type safety and align the code with the OpenTelemetry eBPF profiler v0.139.0 API. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
|
Failing test is from data race fixed #4829 |
|
Noticed that OTEL_VERSION hadn't been updated for 1.11, made a PR to update the release branch. #4833 |
jharvey10
left a comment
There was a problem hiding this comment.
Did some manual, non-exhaustive comparisons through the changes in the PR description, the AI-generated dep review, and the code searching for anomalies. I didn't find anything that stuck out to me.
|
After this merge the following warnings started appearing: This is where they come from: This call in the parent commit isn't happening anymore to populate the variable: |
|
It looks like metrics used to be initialized implicitly by $ git diff f2ff2fc6048c..a00a0ef2a84c -- metrics/metrics.go
diff --git a/metrics/metrics.go b/metrics/metrics.go
index a0a9beb..752847f 100644
--- a/metrics/metrics.go
+++ b/metrics/metrics.go
@@ -14,10 +14,7 @@ import (
log "github.com/sirupsen/logrus"
- "go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/metric"
-
- "go.opentelemetry.io/ebpf-profiler/vc"
)
var (
@@ -44,13 +41,11 @@ var (
metricTypes map[MetricID]MetricType
// OTel metric instrumentation
- meter = otel.Meter("go.opentelemetry.io/ebpf-profiler",
- metric.WithInstrumentationVersion(vc.Version()))
counters = map[MetricID]metric.Int64Counter{}
gauges = map[MetricID]metric.Int64Gauge{}
)
-func init() {
+func Start(meter metric.Meter) {
defs := GetDefinitions()
metricTypes = make(map[MetricID]MetricType, len(defs))
for _, md := range defs { |
|
@bobrik thanks for reporting this! We're looking into the issue. |
* Squashed OTel v0.139.0 * converters * update ebpfs * appease copilot * pyroscope ebpf compatibility * Update ebpf fork with otel v0.139.0 * Refactor Pyroscope eBPF to properly handle container IDs and executable reporters (grafana#4827) This change addresses the container ID handling and symbol resolution in the Pyroscope eBPF profiler by: - Replacing FileObserver with ExecutableReporter in the configuration to better reflect its purpose and enable proper cleanup with type assertions - Moving container ID to the proper layer in the event reporting flow by passing it as a parameter to createProfile instead of storing it in ExtraMeta - Simplifying the type system in tests by using upstream types (libpf.FileID and irsymcache.SourceInfo) instead of wrapper types These changes improve type safety and align the code with the OpenTelemetry eBPF profiler v0.139.0 API. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com> * Update go.mod * Fix jaegerremotesampling test * fix a few other tests * Appease linter * linting and tests * downgrade loadbalancing exporter * Changelog entry * Update OTEL_VERSION --------- Co-authored-by: Christian Simon <simon@swine.de> Co-authored-by: Claude <noreply@anthropic.com>
PR Description
From the upstream changelogs, the following changes are included.
Which issue(s) this PR fixes
Notes to the Reviewer
PR Checklist