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 Prometheus Remote Write Exporter supporting Cortex - factory and config #1544

Merged
merged 22 commits into from
Aug 14, 2020

Conversation

huyan0
Copy link
Member

@huyan0 huyan0 commented Aug 13, 2020

Description:
This PR replaces the close PR #1524. Comments on that PR has been addressed here. This PR is part one of PR #1525.

This PR implements the config, config YAML, and factory structs, the prwExporter struct. It also includes a sample configuration to run this Prometheus remote write exporter. The config and factory structs are fully compatible with the Collector component interfaces.

Link to tracking Issue: #1150
Related issues are:
Metrics aggregation proposal: #1422
Prometheus exporter not functional: #1255
Related spec discussion: #731

Documentation:

cc: @huyan0 @alolita @markcartertm @sonofachamp @morigs
Request for review: @jmacd @bogdandrutu @annanay25 @gouthamve @open-telemetry/collector-approvers @open-telemetry/collector-maintainers

@huyan0
Copy link
Member Author

huyan0 commented Aug 13, 2020

Regarding the comments on headers in PR #1524, I couldn't find a headers field inside HTTPClientSettings to merge our headers into; would appreciate some help on that part : )

@codecov
Copy link

codecov bot commented Aug 13, 2020

Codecov Report

Merging #1544 into master will increase coverage by 0.03%.
The diff coverage is 96.92%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1544      +/-   ##
==========================================
+ Coverage   91.42%   91.46%   +0.03%     
==========================================
  Files         248      250       +2     
  Lines       17219    17284      +65     
==========================================
+ Hits        15743    15808      +65     
  Misses       1064     1064              
  Partials      412      412              
Impacted Files Coverage Δ
exporter/prometheusremotewriteexporter/factory.go 95.55% <95.55%> (ø)
exporter/prometheusremotewriteexporter/exporter.go 100.00% <100.00%> (ø)
translator/internaldata/resource_to_oc.go 87.50% <0.00%> (+2.08%) ⬆️

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 e68440e...cc53cf1. Read the comment docs.

@bogdandrutu
Copy link
Member

I said that you should help us with that if you don't mind :). So please create a PR that adds Headers to the Http config :)

If you don't have time probably just file an issue and point to this file to resolve

@huyan0
Copy link
Member Author

huyan0 commented Aug 14, 2020

I said that you should help us with that if you don't mind :). So please create a PR that adds Headers to the Http config :)

If you don't have time probably just file an issue and point to this file to resolve

Ah I see, thanks for the clarification. I will look into that and create a PR.

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

Looks good some minor comments

exporter/prometheusremotewriteexporter/README.md Outdated Show resolved Hide resolved
go.mod Outdated Show resolved Hide resolved
Add explanation to namespace
Remove unused dependency
@huyan0
Copy link
Member Author

huyan0 commented Aug 14, 2020

Hi, I have updated the documents based on comments. I will leave the headers in config for now and remove them in the HTTPClientConfig PR.

go.sum Outdated Show resolved Hide resolved
@huyan0 huyan0 force-pushed the factory_config branch 2 times, most recently from 39ec18e to fbdbf06 Compare August 14, 2020 14:54
fix format in README
@bogdandrutu bogdandrutu merged commit 632dcf0 into open-telemetry:master Aug 14, 2020
MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021
...and propagation.HeaderCarrier to adapt http.Header to this interface.

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
…y#1544)

Bumps [boto3](https://github.com/boto/boto3) from 1.22.7 to 1.22.10.
- [Release notes](https://github.com/boto/boto3/releases)
- [Changelog](https://github.com/boto/boto3/blob/develop/CHANGELOG.rst)
- [Commits](boto/boto3@1.22.7...1.22.10)

---
updated-dependencies:
- dependency-name: boto3
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this pull request Jul 5, 2024
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.

2 participants