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 F5 Cloud Exporter (1/3) #1965

Merged
merged 3 commits into from
Jan 19, 2021
Merged

Add F5 Cloud Exporter (1/3) #1965

merged 3 commits into from
Jan 19, 2021

Conversation

gramidt
Copy link
Member

@gramidt gramidt commented Jan 10, 2021

Description:
Allow exporting of metric, trace, and log data to F5 Cloud

Link to tracking Issue:
#1964

Testing:
Unit tests are included

Documentation:
The documentation includes a short description and the configuration.

Notes:

  • This exporter is currently just a wrapper around the opentelemetry-collector/otlphttp exporter, but includes F5 Cloud specific authentication. (Similar to how the awsprometheusremotewriteexporter is implemented)
  • While this exporter is initially just an authentication wrapper for the otlphttp exporter, additional F5 Cloud specific metadata will be added in the near future.
  • This is the first of 3 PRs. The second will include updates to the code owners and dependabot, and the third will add it to the binary. I'm happy to break this PR up too, but figured since it's just currently a wrapper around the OTLP exporter with some custom authentication logic, it would be pretty easy to review as is.

@codecov
Copy link

codecov bot commented Jan 11, 2021

Codecov Report

Merging #1965 (d8a5963) into master (ebd62bd) will increase coverage by 1.23%.
The diff coverage is 95.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1965      +/-   ##
==========================================
+ Coverage   89.15%   90.39%   +1.23%     
==========================================
  Files         393      397       +4     
  Lines       19329    19469     +140     
==========================================
+ Hits        17233    17599     +366     
+ Misses       1641     1407     -234     
- Partials      455      463       +8     
Flag Coverage Δ
integration 69.73% <ø> (?)
unit 89.18% <95.31%> (+0.02%) ⬆️

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

Impacted Files Coverage Δ
exporter/f5cloudexporter/factory.go 90.32% <90.32%> (ø)
exporter/f5cloudexporter/auth.go 100.00% <100.00%> (ø)
exporter/f5cloudexporter/config.go 100.00% <100.00%> (ø)
internal/common/testing/container/container.go 73.68% <0.00%> (ø)
processor/groupbytraceprocessor/event.go 96.80% <0.00%> (+0.79%) ⬆️
receiver/jmxreceiver/receiver.go 94.80% <0.00%> (+16.88%) ⬆️
receiver/dockerstatsreceiver/docker.go 92.30% <0.00%> (+39.05%) ⬆️
... and 6 more

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 ebd62bd...d8a5963. Read the comment docs.

@gramidt gramidt marked this pull request as ready for review January 11, 2021 05:55
@gramidt gramidt requested a review from a team January 11, 2021 05:55
@gramidt
Copy link
Member Author

gramidt commented Jan 11, 2021

@anuraaga - We may need to start looking into and resolve the linting issue with the spanmetricsprocessor .

processor.go:58:123: newProcessor - result 1 (error) is always nil (unparam)
func newProcessor(logger *zap.Logger, config configmodels.Exporter, nextConsumer consumer.TracesConsumer) (*processorImp, error) {

@gramidt
Copy link
Member Author

gramidt commented Jan 11, 2021

@anuraaga - Appears that the linting issue with spanmetricsprocessor has been addressed with the latest commits to master.

@gramidt
Copy link
Member Author

gramidt commented Jan 13, 2021

Happy Wednesday/Thursday, @anuraaga!

Hope your week is going well! Is there any possibility that you may be able to review this today or tomorrow? If not, do you have any recommendations on another reviewer that may be able to review the PR?

@anuraaga
Copy link
Contributor

@gramidt Sorry for missing this! My bad just seemed to be missing the notification, will check it out

@gramidt
Copy link
Member Author

gramidt commented Jan 15, 2021

No worries at all, @anuraaga! I know you're busy and greatly appreciate all of your help.

exporter/f5cloudexporter/README.md Outdated Show resolved Hide resolved
exporter/f5cloudexporter/README.md Outdated Show resolved Hide resolved
exporter/f5cloudexporter/README.md Outdated Show resolved Hide resolved
exporter/f5cloudexporter/README.md Outdated Show resolved Hide resolved
exporter/f5cloudexporter/config.go Show resolved Hide resolved
@gramidt
Copy link
Member Author

gramidt commented Jan 15, 2021

@anuraaga - Just updated with your current list of suggestions. Thank you again for all of your help!

@gramidt
Copy link
Member Author

gramidt commented Jan 15, 2021

@bogdandrutu - I know you're super busy preparing for the workshop tomorrow, but if you get a moment, I would greatly appreciate your help with getting this merged. Thank you in advance for all of your help!

@bogdandrutu bogdandrutu merged commit a8b133b into open-telemetry:master Jan 19, 2021
@gramidt gramidt deleted the f5cloudexporter-imp branch January 20, 2021 20:15
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.

3 participants