-
Notifications
You must be signed in to change notification settings - Fork 2
New - Upgrade OpenTelemetry Dependencies and Fix Declarative Configuration #413
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR upgrades OpenTelemetry dependencies (Java Agent 2.23.0→2.24.0, SDK 1.57.0→1.58.0, ByteBuddy 1.15.10→1.18.4, Java Contrib 1.52.0→1.53.0-alpha) and refactors declarative configuration to use strongly-typed property models instead of raw maps, aligning with API changes in the new versions.
Changes:
- Replaced
Collections.emptyMap()andMap<String, Object>with typed property model classes for declarative configuration - Updated metric export defaults (temporality_preference: cumulative→delta, default_histogram_aggregation: explicit_bucket_histogram→base2_exponential_bucket_histogram)
- Updated
IgnoredTypesConfigurerinterface signature to remove unusedConfigPropertiesparameter
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| dependencyManagement/build.gradle.kts | Updates OpenTelemetry and ByteBuddy dependency versions |
| libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProvider.java | Replaces raw maps with typed property models and updates metric export configuration values |
| libs/shared/src/main/java/com/solarwinds/opentelemetry/extensions/SolarwindsIgnoredTypesConfigurer.java | Removes unused ConfigProperties parameter from configure method |
| custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/provider/CustomConfigCustomizerProvider.java | Replaces Collections.emptyMap() with typed property models for resource detectors and processors |
| custom/src/main/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoader.java | Refactors configuration loading to use ExtendedOpenTelemetry API instead of ConfigProvider |
| libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/config/provider/SharedConfigCustomizerProviderTest.java | Updates tests to use typed property models and validates new metric configuration defaults |
| libs/shared/src/test/java/com/solarwinds/opentelemetry/extensions/MetricExporterCustomizerTest.java | Refactors test to use mocks and verify behavior instead of object inequality |
| custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/SpanExporterCustomizerTest.java | Refactors test to use mocks and verify setProxy call |
| custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/LogRecordExporterCustomizerTest.java | Refactors test to use mocks and verify setProxyOptions call |
| custom/src/test/java/com/solarwinds/opentelemetry/extensions/config/DeclarativeLoaderTest.java | Adds new test coverage for declarative configuration parsing |
| README.md | Updates OTEL version badge from 2.3.0 to 2.24.0 |
cheempz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! left a couple questions :)
| new PushMetricExporterPropertyModel() | ||
| .withAdditionalProperty("timeout", 10000) | ||
| .withAdditionalProperty("protocol", "grpc") | ||
| .withAdditionalProperty("compression", "gzip") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be a premature question since we haven't officialy introduce declarative config for APM Java--just noticed here the protocol is set to grpc. For the proxy config feature #345 we switched to http/proto, so wondering:
- should we set it to same here?
- if a customer has both proxy config and uses declarative file, would the settings here like compression, temporality, etc be carried over to the customized proxy-enabled exporter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- we should
- nope. we'll need to make adjustment for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i'll make a new pr to take of those
🎯 Overview
This PR upgrades OpenTelemetry dependencies to the latest versions and refactors declarative configuration implementation to align with API changes in OpenTelemetry SDK 1.58.0 and Java Instrumentation 2.24.0.
📦 Dependency Updates
Core Dependencies
2.23.0→2.24.01.57.0→1.58.01.15.10→1.18.41.52.0-alpha→1.53.0-alpha🔧 Key Changes
1. Declarative Configuration Refactoring
Introduction of Typed Property Models
Replaced usage of
Collections.emptyMap()and rawMap<String, Object>with strongly-typed property model classes:ExperimentalResourceDetectorPropertyModelSpanProcessorPropertyModelSamplerPropertyModelPushMetricExporterPropertyModelExperimentalLanguageSpecificInstrumentationPropertyModelImpact: Improves type safety and aligns with OpenTelemetry SDK's declarative configuration API changes.
Example Transformation
Before:
After:
2. Metric Export Configuration Updates
Updated default metric export settings in
SharedConfigCustomizerProvider:temporality_preferencecumulativedeltadefault_histogram_aggregationexplicit_bucket_histogrambase2_exponential_bucket_histogram3. API Signature Updates
IgnoredTypesConfigurer Interface
Updated method signature to match OpenTelemetry API changes:
4. Test Improvements
New Test Coverage
Added comprehensive unit tests for
DeclarativeLoaderto validate:beforeAgenthookEnhanced Mock-Based Testing
Updated exporter customizer tests to use proper mocking instead of creating real instances:
Improvements:
setProxyOptions()/setProxy()calls instead of object inequalityBefore:
After:
Test Data Refactoring
Test services data