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

sumologicexporter: add graphite format #2695

Merged

Conversation

sumo-drosiek
Copy link
Member

Description:

  • Add support for graphite format to sumo logic exporter
  • Add sanitization for carbon2 format
  • Replace = with : for sanitization

Link to tracking Issue:
#1498

Testing:
Unit tests, manual tests

Documentation:
Code comments

Dominik Rosiek added 3 commits March 15, 2021 11:13
@sumo-drosiek sumo-drosiek requested a review from a team March 15, 2021 14:35
@sumo-drosiek
Copy link
Member Author

cc: @pmalek-sumo @pmm-sumo

@codecov
Copy link

codecov bot commented Mar 15, 2021

Codecov Report

Merging #2695 (f2b0eec) into main (5efd35d) will increase coverage by 0.02%.
The diff coverage is 95.69%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2695      +/-   ##
==========================================
+ Coverage   91.37%   91.40%   +0.02%     
==========================================
  Files         433      434       +1     
  Lines       21530    21617      +87     
==========================================
+ Hits        19674    19758      +84     
- Misses       1390     1391       +1     
- Partials      466      468       +2     
Flag Coverage Δ
integration 69.18% <ø> (+0.24%) ⬆️
unit 90.31% <95.69%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
exporter/sumologicexporter/config.go 100.00% <ø> (ø)
exporter/sumologicexporter/exporter.go 80.00% <90.90%> (+1.25%) ⬆️
exporter/sumologicexporter/graphite_formatter.go 96.42% <96.42%> (ø)
exporter/sumologicexporter/carbon_formatter.go 100.00% <100.00%> (ø)
exporter/sumologicexporter/factory.go 71.42% <100.00%> (+0.84%) ⬆️
exporter/sumologicexporter/fields.go 100.00% <100.00%> (ø)
exporter/sumologicexporter/sender.go 90.69% <100.00%> (+0.27%) ⬆️
receiver/carbonreceiver/transport/tcp_server.go 67.00% <0.00%> (+1.00%) ⬆️

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 5efd35d...f2b0eec. Read the comment docs.

@tigrannajaryan
Copy link
Member

@pmm-sumo please review.

- `source_category` (optional): Desired source category. Useful if you want to override the source category configured for the source.
Placeholders `%{attr_name}` will be replaced with attribute value for `attr_name`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is not entirely clear. Perhaps it should be marked as graphite-only or so?

Copy link
Member Author

Choose a reason for hiding this comment

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

you mean graphite_template?

Copy link
Member Author

Choose a reason for hiding this comment

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

source_category is independent of metric type

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, right, then I would rephrase the whole section that refers to it so it's clear what's going on. Perhaps something akin to:

You can specify a template with an attribute for source_category, source_name, source_host or graphite_template using %{attr_name}. For example, %{my_attr}, when there is an attribute my_attr: my_value would be expanded to my_value.

And then refer to it for each attribute, like:

An attribute can be referred using %{attr_name}

Copy link
Contributor

@pmm-sumo pmm-sumo left a comment

Choose a reason for hiding this comment

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

I think it would be good to rephrase how attribute names are being expanded in README.md

@bogdandrutu
Copy link
Member

@pmm-sumo please approve when you feel good about this PR so I can take a final look and merge

Signed-off-by: Dominik Rosiek <drosiek@sumologic.com>
@pmm-sumo
Copy link
Contributor

@pmm-sumo please approve when you feel good about this PR so I can take a final look and merge

Thank you @bogdandrutu , I think we are good to merge now

@github-actions
Copy link
Contributor

github-actions bot commented Apr 6, 2021

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@tigrannajaryan tigrannajaryan merged commit bbab39d into open-telemetry:main Apr 14, 2021
bogdandrutu referenced this pull request in bogdandrutu/opentelemetry-collector-contrib Apr 14, 2021
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
tigrannajaryan pushed a commit that referenced this pull request Apr 14, 2021
#3102)

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
pmatyjasek-sumo referenced this pull request in pmatyjasek-sumo/opentelemetry-collector-contrib Apr 28, 2021
 - Add support for graphite format to sumo logic exporter
 - Add sanitization for carbon2 format
 - Replace `=` with `:` for sanitization

**Link to tracking Issue:**
#1498
pmatyjasek-sumo referenced this pull request in pmatyjasek-sumo/opentelemetry-collector-contrib Apr 28, 2021
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.

5 participants