Skip to content
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

Update support for semconv versions up to semconv 1.27.0 #440

Merged
merged 1 commit into from
Feb 10, 2025

Conversation

simitt
Copy link
Contributor

@simitt simitt commented Feb 7, 2025

The goal of this PR is to support the latest semantic conventions for already mapped otel attributes, without introducing any breaking changes. This is desirable so that users can upgrade their agent SDKs to newer versions without losing semantics when data are mapped with this library.
Backwards compatibility is achieved by keeping the deprecated field names in the mapping. Where possible, the new field names were added. In cases where no new field names are defined by otel, the fields would be stored as labels instead of concrete mappings, however this would be a breaking change in the semantic conventions itself, rather than in the apm-data mapping logic.

The mapping logic is taken from https://opentelemetry.io/schemas/1.27.0 as much as possible, and otherwise where possible from the deprecation notes in the semconv version implementations.

This PR does not introduce support for additional, previously unmapped otel fields.

@simitt simitt requested a review from a team as a code owner February 7, 2025 16:30
@elastic-observability-automation elastic-observability-automation bot added the safe-to-test Changes are safe to run in the CI label Feb 7, 2025
@simitt simitt requested a review from lahsivjar February 7, 2025 16:31
}, map[string]interface{}{
"http.scheme": "https",
"http.server_name": "testing.invalid",
"server.port": 80,
Copy link
Contributor

Choose a reason for hiding this comment

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

QQ: When I was working with the user fields mapping, I wondered if the unit tests file should be using constants for these. Previously, they were defined privately in otlp package while the tests were in otlp_test, so I couldn't just import it and use the constants. Moving forward, should we use semconv constants where possible even in tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No strong opinion, but using hardcoded values instead of updating the tests to the same constants as the actual business logic, allows to more easily spot if any previously supported key is dropped or e.g. accidentally renamed.
Overall, I do not expect us to put a lot of effort into this going forward. We should keep up-to-date with semconv, but the update PRs should hopefully be small as semconv stabilizes more and when we upgrade more regularly (this time we upgraded from 1.5 to 1.27.

@simitt simitt merged commit 1e73b0c into elastic:main Feb 10, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe-to-test Changes are safe to run in the CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants