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

Add collector example #657

Merged
merged 12 commits into from
Jul 7, 2023
Merged

Conversation

psx95
Copy link
Contributor

@psx95 psx95 commented Jul 5, 2023

This PR adds OpenTelemetry Collector based example for metrics export.

Also fixes misconfigured metrics-dashboard json

@psx95 psx95 requested a review from a team as a code owner July 5, 2023 19:48
@codecov
Copy link

codecov bot commented Jul 5, 2023

Codecov Report

Merging #657 (691dea5) into main (cc91e37) will decrease coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #657      +/-   ##
==========================================
- Coverage   68.47%   68.39%   -0.09%     
==========================================
  Files          36       36              
  Lines        4559     4559              
==========================================
- Hits         3122     3118       -4     
- Misses       1284     1288       +4     
  Partials      153      153              

see 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@damemi damemi left a comment

Choose a reason for hiding this comment

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

Please also update the gitignore to exclude the new example/metric/collector/metrics binary

Comment on lines 26 to 29
1. Exporting metrics via the SDK.
2. Exporting metrics via the OpenTelemetry Collector.

Change the current directory to the example you wish to run - either to `sdk` directory or `collector` directory and then run the example using following commands:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. Exporting metrics via the SDK.
2. Exporting metrics via the OpenTelemetry Collector.
Change the current directory to the example you wish to run - either to `sdk` directory or `collector` directory and then run the example using following commands:
1. Exporting metrics via the [SDK](sdk).
2. Exporting metrics via the [OpenTelemetry Collector](collector).
Change the current directory to the example you wish to run - either to [`sdk`](sdk) directory or [`collector`](collector) directory and then run the example using following commands:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the text "SDK" and "OpenTelemetry Collector" in the bullet points should not be a hyperlink to the folders since they are actually intended to explain what component is being used to export metrics.

I will add hyperlinks in the instructions though.

example/metric/collector/sample-collector-config.yaml Outdated Show resolved Hide resolved
example/metric/collector/sample-collector-config.yaml Outdated Show resolved Hide resolved
@psx95
Copy link
Contributor Author

psx95 commented Jul 5, 2023

Only the files in example directory are changed. Coverage calculation should not include these files.

@psx95 psx95 requested a review from damemi July 5, 2023 22:33
Copy link
Member

@damemi damemi left a comment

Choose a reason for hiding this comment

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

only suggestion would be to just drop the debugging exporters, otherwise lgtm

example/metric/collector/sample-collector-config.yaml Outdated Show resolved Hide resolved
@psx95 psx95 merged commit a19d9ea into GoogleCloudPlatform:main Jul 7, 2023
24 of 25 checks passed
@psx95 psx95 deleted the add-collector-example branch July 7, 2023 15:49
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.

None yet

2 participants