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

Enable resource attributes to metric label conversion option #1552

Merged
merged 1 commit into from
Nov 11, 2020

Conversation

hossain-rayhan
Copy link
Contributor

Description:
Recently a PR 2060 was merged in the core opentelemetry-collector repo for converting resource attributes to metric labels. This was added as a utility option in the exporterhelper module so that any exporter can easily opt-in. We need to enable this settings for awsemf exporter so that customer can enable it if they want.

This is a required option for AWS ECS Container Metrics customers.

This change introduces a new function for creating the componenet.MetricsExporter which utilizes the exporterhelper module with resource_to_telemetry_conversion option as well as our pre-existing New() function.

Link to tracking Issue:
#1551

Testing:
It passes all the existing unit tests.
Added new unit tests for testing the new changes.
Manually tested for e2e experience.

Documentation:
README updated.

Signed-off-by: Rayhan Hossain <hossain.rayhan@outlook.com>
@hossain-rayhan
Copy link
Contributor Author

Tagging @mxiamxia @shaochengwang @wyTrivail for visibility.

@codecov
Copy link

codecov bot commented Nov 11, 2020

Codecov Report

Merging #1552 (6a68423) into master (33bba14) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1552      +/-   ##
==========================================
+ Coverage   89.40%   89.41%   +0.01%     
==========================================
  Files         354      354              
  Lines       17313    17324      +11     
==========================================
+ Hits        15478    15490      +12     
+ Misses       1372     1371       -1     
  Partials      463      463              
Flag Coverage Δ
integration 71.46% <ø> (ø)
unit 88.04% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
exporter/awsemfexporter/emf_exporter.go 97.91% <100.00%> (+0.26%) ⬆️
exporter/awsemfexporter/factory.go 100.00% <100.00%> (ø)
processor/groupbytraceprocessor/event.go 96.77% <0.00%> (+0.80%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33bba14...6a68423. Read the comment docs.

Copy link
Member

@mxiamxia mxiamxia left a comment

Choose a reason for hiding this comment

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

LGTM! Ty!

@bogdandrutu bogdandrutu merged commit 994cabe into open-telemetry:master Nov 11, 2020
dyladan referenced this pull request in dynatrace-oss-contrib/opentelemetry-collector-contrib Jan 29, 2021
* Add headers in HTTPClientSettings

* add helper

* Revert "add helper"

This reverts commit 394f94f8e1c5dae11d67166030fcb718da665862.

* remove testutil_test

* add interceptor only when there are headers

* add documentation

fix TestAllHTTPClientSettings
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
* Update license-check to ignore all vendor dirs

* Remove vendor path exclude from license-check find
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.

4 participants