-
Notifications
You must be signed in to change notification settings - Fork 2.8k
signaltometricsconnector: support gauges #40113
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
signaltometricsconnector: support gauges #40113
Conversation
Add support for gauges with last value aggregation. relates to open-telemetry#37093
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.
Added a few discussion points on the supported return types for the value
expression.
connector/signaltometricsconnector/internal/aggregator/aggregator.go
Outdated
Show resolved
Hide resolved
connector/signaltometricsconnector/internal/aggregator/aggregator.go
Outdated
Show resolved
Hide resolved
- allow for all signals - only support numbers in OTTL expressions rather than Grok maps. - move check into loop
@lahsivjar thanks for the initial round of feedback, moving this out of draft now after addressing your feedback. |
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, a few minor README comments! Thanks for addressing all the review points.
if strings.Contains(mi.Gauge.Value, "ExtractGrokPatterns") { | ||
// Ensure a [key] selector is present after ExtractGrokPatterns | ||
if !regexp.MustCompile(`ExtractGrokPatterns\([^)]*\)\s*\[[^\]]+\]`).MatchString(mi.Gauge.Value) { | ||
return fmt.Errorf("ExtractGrokPatterns: a single key selector[key] is required for signal to gauge") | ||
} | ||
} |
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.
This check would be true for all configured metric types so maybe in a follow-up PR we can generalize this as a common validation for ExtractGrokPatterns
.
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.
This is not that important though and we can just create an issue and leave it for future too as we might need to think about how to (or if to) handle config validation for all funcs that return map (or other non-primitives)
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.
created #40118
Co-authored-by: Vishal Raj <[email protected]>
@simitt There are some minor lint errors and it also needs a changelog. More details on adding changelog is available here: opentelemetry-collector-contrib/CONTRIBUTING.md Lines 72 to 76 in c37a07f
|
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Add support for parsing `gauges` from any signal type with last value aggregation per batch. For now only numerical values are supported. Parsing timestamps is not yet supported. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Relates to open-telemetry#37093 <!--Describe what testing was performed and which tests were added.--> #### Testing Automated unit tests have been added. <!--Describe the documentation added.--> #### Documentation README was updated <!--Please delete paragraphs that you did not use before submitting.--> --------- Co-authored-by: Vishal Raj <[email protected]> Co-authored-by: Christos Markou <[email protected]>
Description
Add support for parsing
gauges
from any signal type with last value aggregation per batch.For now only numerical values are supported. Parsing timestamps is not yet supported.
Link to tracking issue
Relates to #37093
Testing
Automated unit tests have been added.
Documentation
README was updated