-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
[extension/encoding/awscloudwatchmetricstreams_encoding] Support unmarshalling JSON-formatted CloudWatch Metric Streams #38419
[extension/encoding/awscloudwatchmetricstreams_encoding] Support unmarshalling JSON-formatted CloudWatch Metric Streams #38419
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.
Thanks @constanca-m! Looking good, main concerns are to do with how the tests are implemented.
extension/encoding/awscloudwatchmetricstreamsencodingextension/json_unmarshaler_test.go
Outdated
Show resolved
Hide resolved
extension/encoding/awscloudwatchmetricstreamsencodingextension/json_unmarshaler_test.go
Outdated
Show resolved
Hide resolved
extension/encoding/awscloudwatchmetricstreamsencodingextension/json_unmarshaler.go
Outdated
Show resolved
Hide resolved
extension/encoding/awscloudwatchmetricstreamsencodingextension/json_unmarshaler_test.go
Outdated
Show resolved
Hide resolved
Co-authored-by: Andrew Wilkins <axwalk@gmail.com>
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.
Looks great, thanks!
// joinMetricsFromFile reads the metrics inside the files, | ||
// and joins them in the format a record expects it to be: | ||
// each metric is expected to be in 1 line, and every new | ||
// line marks a new metric |
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.
Nice, I like this approach.
@open-telemetry/collector-contrib-approvers Can I have a review here? |
@constanca-m Can you fix the CI issues? |
@mx-psi Took a few attempts, but the CI issues are all gone now. |
@MovieStoreGuy can we merge this despite the |
Description
This PR implements the translation of JSON-formatted CloudWatch Metric Streams into OpenTelemetry metrics.
It receives a record, and splits that record by each new line. Each of these parts will correspond to a JSON CloudWatch Metric Stream. It then translates this metric into an OpenTelemetry metric. The obtained metrics from the record are returned.
It follows the same logic as https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/receiver/awsfirehosereceiver/internal/unmarshaler/cwmetricstream.
Link to tracking issue
Fixes #38407.
Testing
I have added unit tests to:
You can easily read and study what we get to what we return in the
testdata/json
files of the extension.Documentation
The coding comments and the README file should be enough for documentation.