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

Humio exporter init #3022

Merged
merged 15 commits into from
Apr 13, 2021
Merged

Conversation

Xitric
Copy link
Contributor

@Xitric Xitric commented Apr 8, 2021

Description:
This is the first PR for the Humio exporter, containing readme, configuration, factory, and relevant tests.
The first series of PRs will focus on implementing exporting of traces, with logs to follow soon.

Apologies for the large PR, but I hope that the changes are sufficiently trivial.

Link to tracking Issue:
#3021

Testing:

  • config_test.go: Tests that the configuration options can be correctly loaded, including defaults. Also tests configuration sanitization.
  • factory-test.go: Tests that a factory only creates a new exporter (currently a stub) if provided with a valid configuration.

Documentation:
Added documentation for the purpose of the exporter and all its current configuration options. This includes Humio endpoints, ingest tokens, and storage options for Humio.

Also added examples of configuring the exporter at various degrees of detail.

Lastly, I added references to configuration options inherited from:

@Xitric Xitric requested a review from a team April 8, 2021 14:52
@codecov
Copy link

codecov bot commented Apr 8, 2021

Codecov Report

Merging #3022 (2b7631b) into main (0c198ee) will increase coverage by 0.04%.
The diff coverage is 97.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3022      +/-   ##
==========================================
+ Coverage   91.55%   91.59%   +0.04%     
==========================================
  Files         464      467       +3     
  Lines       22846    22921      +75     
==========================================
+ Hits        20916    20994      +78     
+ Misses       1437     1436       -1     
+ Partials      493      491       -2     
Flag Coverage Δ
integration 68.96% <ø> (-0.07%) ⬇️
unit 90.56% <97.33%> (+0.04%) ⬆️

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

Impacted Files Coverage Δ
exporter/humioexporter/config.go 94.11% <94.11%> (ø)
exporter/humioexporter/factory.go 100.00% <100.00%> (ø)
exporter/humioexporter/traces_exporter.go 100.00% <100.00%> (ø)
exporter/signalfxexporter/dimensions/requests.go 92.15% <0.00%> (+9.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 0c198ee...2b7631b. Read the comment docs.

To make the test coverage check pass
@Xitric
Copy link
Contributor Author

Xitric commented Apr 12, 2021

@jpkrohling I am not trying to exert any pressure, just wanted to make sure this PR does not drown, since I can see I have made it to the second page over the weekend.

I added some placeholder tests to the unimplemented parts of the trace exporter to satisfy the code coverage check. All the checks are passing now.

@jpkrohling
Copy link
Member

Thanks for pinging me, I would have missed it otherwise. I'll check it soon.

exporter/humioexporter/README.md Outdated Show resolved Hide resolved
exporter/humioexporter/README.md Outdated Show resolved Hide resolved
exporter/humioexporter/README.md Outdated Show resolved Hide resolved
exporter/humioexporter/README.md Outdated Show resolved Hide resolved
exporter/humioexporter/README.md Outdated Show resolved Hide resolved
exporter/humioexporter/config.go Outdated Show resolved Hide resolved
exporter/humioexporter/config.go Outdated Show resolved Hide resolved
exporter/humioexporter/config.go Show resolved Hide resolved
exporter/humioexporter/factory.go Outdated Show resolved Hide resolved
exporter/humioexporter/go.mod Outdated Show resolved Hide resolved
@jpkrohling
Copy link
Member

Approved. I had a few comments that might be addressed in the next PR.

@Xitric
Copy link
Contributor Author

Xitric commented Apr 12, 2021

Thank you for the review and comments. I will make sure to address them all in the next PR.

@jpkrohling
Copy link
Member

@tigrannajaryan, @bogdandrutu would one of you please merge this one? Despite my comments, this is approved and I trust @Xitric is addressing them in the next PR.

@Xitric
Copy link
Contributor Author

Xitric commented Apr 13, 2021

@tigrannajaryan I see there were some big changes to configurations, so I understand why this could not wait ;)

I figured that I might as well address all the comments at once. I have updated the tests as appropriate, and everything seems to be working again. Thank you for the thorough code reviews, it is greatly appreciated!

@tigrannajaryan tigrannajaryan merged commit e9f085b into open-telemetry:main Apr 13, 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 pushed a commit to pmatyjasek-sumo/opentelemetry-collector-contrib that referenced this pull request Apr 28, 2021
This is the first PR for the Humio exporter, containing readme, configuration, factory, and relevant tests.
The first series of PRs will focus on implementing exporting of traces, with logs to follow soon.

Apologies for the large PR, but I hope that the changes are sufficiently trivial.

**Link to tracking Issue:**
open-telemetry#3021

**Testing:**
- `config_test.go`: Tests that the configuration options can be correctly loaded, including defaults. Also tests configuration sanitization.
- `factory-test.go`: Tests that a factory only creates a new exporter (currently a stub) if provided with a valid configuration.

**Documentation:**
Added documentation for the purpose of the exporter and all its current configuration options. This includes Humio endpoints, ingest tokens, and storage options for Humio.

Also added examples of configuring the exporter at various degrees of detail.

Lastly, I added references to configuration options inherited from:
- [HTTP Client Configuration](https://github.com/open-telemetry/opentelemetry-collector/tree/main/config/confighttp#client-configuration)
- [TLS Configuration](https://github.com/open-telemetry/opentelemetry-collector/blob/main/config/configtls/README.md#tls-configuration-settings)
- [Queueing, Retry, and Timeout Configuration](https://github.com/open-telemetry/opentelemetry-collector/blob/main/exporter/exporterhelper/README.md#configuration)
pmatyjasek-sumo pushed a commit to pmatyjasek-sumo/opentelemetry-collector-contrib that referenced this pull request 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.

3 participants