-
Notifications
You must be signed in to change notification settings - Fork 422
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
refactor: prometheus attributes format to otel attributes #4188
Merged
adamw
merged 12 commits into
softwaremill:master
from
varshith257:refactor/OTEL-attributes
Dec 13, 2024
+93
−38
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
2e66c62
refactor: prometheus attributes format to otel attributes
varshith257 28e66df
Update OpenTelemetryMetrics.scala
varshith257 2347ca3
refactor!: prometheus and OTEL defaul Metric formats
varshith257 23553cd
feat!: add review comments
varshith257 a95853a
feat: add req additional attributes
varshith257 efe8e7d
feat: move OTEL metrics related code to OTEL Module
varshith257 f0cd974
cleanup
varshith257 43ebc64
Try to return an Option[String] with review suggestions
varshith257 25b1958
Fix compilation error
varshith257 88af099
Update OpenTelemetryMetrics.scala
varshith257 a3ebd6c
feat: handle conditional attributes in OpenTelemetryMetrics
varshith257 97ef032
Refactor and align metrics handling with optional labels
varshith257 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Does we need to account
Option[String]
for Zio Metrics ig it may account for other metrics too?Or better strategy to move this logic to OpenTelemetry module and introduce option labels than affecting other modules
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.
that's fine, let's just have optional labels. This might be useful for ZIO as well