Skip to content

Broad app info improvements (required for OpenTelemetry metrics) #1082

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

Merged
merged 4 commits into from
Jun 12, 2024

Conversation

rowanmanning
Copy link
Member

These are all required to add OpenTelemetry metrics, we need to solidify the app info properties that we rely on otherwise our metrics are rejected. This does the following:

  • Firms up the releaseVersion property, required in our metrics
  • Adds in an instanceId property, required in our metrics
  • Adds aliases for OpenTelemetry semantic conventions, which is the first step towards adopting them fully. We'll deprecate old property names in future (resolves Align app info properties with OpenTelemetry #818)

@rowanmanning rowanmanning requested a review from a team as a code owner June 11, 2024 09:59
@rowanmanning rowanmanning changed the title Broad app info improvements Broad app info improvements (required for OpenTelemetry metrics) Jun 11, 2024
Copy link
Member

@alexmuller alexmuller left a comment

Choose a reason for hiding this comment

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

Nice, looks good.

When appInfo.instanceId is a random UUID it is reused across the instantiation of appInfo which is the behaviour we want - in my opinion. Every application restart will create a new UUID but the UUID will be consistent between restarts.

@rowanmanning
Copy link
Member Author

Yep, this is what EDO and OpenTelemetry advices as well, if there is no real instance ID then it should be unique to the running process. We may be able to find other IDs in environment variables later for different platforms

This makes our calculation of system version a lot more solid. This is
going to be required for OpenTelemetry metrics, because any metric sent
without a version will end up in the Dead Letter metrics. Because of
this I want to give users of Reliability Kit more of a guarantee that
their metrics will arrive.
This adds aliases for a bunch of app-info properties that align with
OpenTelemetry's Semantic Conventions. This closes #818.
This is going to be required to send OpenTelemetry metrics as our
systems will only accept metrics with the corresponding semantic
resource attribute.
@rowanmanning rowanmanning force-pushed the broad-app-info-improvements branch from a75abcb to f64c538 Compare June 12, 2024 12:46
@rowanmanning rowanmanning enabled auto-merge (rebase) June 12, 2024 12:47
@rowanmanning rowanmanning merged commit b43aab0 into main Jun 12, 2024
13 checks passed
@rowanmanning rowanmanning deleted the broad-app-info-improvements branch June 12, 2024 12:48
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.

Align app info properties with OpenTelemetry
2 participants