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.

Sign up for GitHub

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

Jump to bottom

Update prom rw exporter #1359

Merged
merged 27 commits into from
Oct 31, 2022
Merged

Update prom rw exporter #1359

merged 27 commits into from
Oct 31, 2022

Conversation

Seefooo
Copy link
Contributor

@Seefooo Seefooo commented Sep 27, 2022 •
edited
Loading

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes #933

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration

  • Unit tests have been updated and should provide similar coverage of functionality from before. Some tests where replaced, others fixed to support changes to MetricExporter ABC & or new components from OT SDK
  • Example directory for this exporter previously contained a complete example. I updated that to also provide similar coverage of the supported OT metric objects. It works as defined following the README instructions in there. No instruction update was required.

Does This PR Require a Core Repo Change?

  • No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

  • Followed the style guidelines of this project (Ran linters, but may have missed some things)
  • Changelogs have been updated
  • Unit tests have been added
  • Documentation has been updated

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 27, 2022 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@Seefooo Seefooo force-pushed the update_prom_rw_exporter branch from b7e76eb to ce9c090 Compare September 27, 2022 18:41
@Seefooo Seefooo marked this pull request as ready for review September 28, 2022 13:20
@Seefooo Seefooo requested a review from a team September 28, 2022 13:20
@Seefooo
Proto & Tox updates
64634b4 
- add Tox commands to setup & test the exporter
- add the Prometheus Remote Write Proto files & update them.
- added a mini-script that wraps the protoc command to regen. them
@Seefooo
Add some tests & test infra.
6db8824
@Seefooo
Update test cases
b115a46
@Seefooo
More fixes from Example run
ad84c56 
- Fix some naming issues,add a regex to correct these
- Update setup.cfg versions
- Update the example app for newer sdk syntax
@Seefooo
Ran linter
f45a968
@Seefooo
Version update
5c2ad66 
this would be released as part of 1.13.0
@Seefooo Seefooo force-pushed the update_prom_rw_exporter branch from ce9c090 to 5c2ad66 Compare September 28, 2022 13:20
@srikanthccv srikanthccv mentioned this pull request Oct 2, 2022
Closed
Seefooo and others added 4 commits October 4, 2022 11:18
@Seefooo
Update setup.cfg to use >=3.7 instead of 3.8
2483633
@Seefooo
Address automated checks
11b9e8f 
- Fix flake8 issues
- Correct spelling mistakes (except false positive 'ro' in docker
  compose)
- Updated Changelog. Not sure if this is worthy
@srikanthccv
Merge branch 'main' into update_prom_rw_exporter
c5fcf18
@srikanthccv
Merge branch 'main' into update_prom_rw_exporter
c6f1a8f
aabmass
aabmass reviewed Oct 11, 2022
View reviewed changes
Copy link
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

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

Thanks for PR! Just leaving a few comments but overall looks great 😃

exporter/opentelemetry-exporter-prometheus-remote-write/README.rst Outdated Show resolved Hide resolved
exporter/opentelemetry-exporter-prometheus-remote-write/README.rst Outdated Show resolved Hide resolved
exporter/opentelemetry-exporter-prometheus-remote-write/README.rst Outdated Show resolved Hide resolved
exporter/opentelemetry-exporter-prometheus-remote-write/README.rst Outdated Show resolved Hide resolved
exporter/opentelemetry-exporter-prometheus-remote-write/proto/generate-proto-py.sh Show resolved Hide resolved
exporter/opentelemetry-exporter-prometheus-remote-write/setup.cfg Outdated Show resolved Hide resolved
exporter/opentelemetry-exporter-prometheus-remote-write/setup.cfg Outdated Show resolved Hide resolved
...orter-prometheus-remote-write/src/opentelemetry/exporter/prometheus_remote_write/__init__.py Outdated Show resolved Hide resolved
...orter-prometheus-remote-write/src/opentelemetry/exporter/prometheus_remote_write/__init__.py Show resolved Hide resolved
...orter-prometheus-remote-write/src/opentelemetry/exporter/prometheus_remote_write/__init__.py Outdated Show resolved Hide resolved
Seefooo and others added 4 commits October 12, 2022 10:14
@Seefooo
Updates from review
d134a52 
- Update to pyproject.toml & float deps
- Update README.rst
- Updates to automate pulling & generating protobuf files
- Add README for protobuf generation
@Seefooo
Fix the shutdown method
15a03b4
@aabmass
Merge branch 'main' into update_prom_rw_exporter
b1eb757
@Seefooo
Remove extra proto files & update regex
b8e249d
@Seefooo Seefooo mentioned this pull request Oct 18, 2022
Open
aabmass
aabmass approved these changes Oct 18, 2022
View reviewed changes
Copy link
Member

@aabmass aabmass left a comment

Choose a reason for hiding this comment

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

LGTM with just a few changes. Thanks again for contributing :)

exporter/opentelemetry-exporter-prometheus-remote-write/examples/sampleapp.py Outdated Show resolved Hide resolved
exporter/opentelemetry-exporter-prometheus-remote-write/pyproject.toml Outdated Show resolved Hide resolved
...porter-prometheus-remote-write/src/opentelemetry/exporter/prometheus_remote_write/version.py Outdated Show resolved Hide resolved
exporter/opentelemetry-exporter-prometheus-remote-write/tests/conftest.py Outdated Show resolved Hide resolved
...orter-prometheus-remote-write/src/opentelemetry/exporter/prometheus_remote_write/__init__.py Outdated Show resolved Hide resolved
Seefooo
Seefooo commented Oct 20, 2022
View reviewed changes
pyproject.toml Outdated Show resolved Hide resolved
@Seefooo Seefooo force-pushed the update_prom_rw_exporter branch from 9158003 to 4450d3a Compare October 24, 2022 17:11
Seefooo and others added 3 commits October 26, 2022 09:37
@Seefooo
Merge branch 'main' into update_prom_rw_exporter
08028b4
@Seefooo
Undo adding 'gen' to ignore list for black
e83d9b5 
- Was causing issues with other tools. I just ran our formatting tool on
  the generated files to avoid issues.
- add a gitignore here to ensure our mimicked import setup doesn't get
  committed.
@Seefooo
Merge branch 'main' into update_prom_rw_exporter
b0c184f
@srikanthccv srikanthccv self-assigned this Oct 27, 2022
Seefooo and others added 4 commits October 28, 2022 10:20
@Seefooo
Fixes from pylint output
54ac259 
- Cannot fix the syntax failures on 'examples/*' directory tho...
@Seefooo
Move to examples dir to example
e0f25d2 
- avoids the pylint issue. This is probably better anyway as the example
  is focused on setting up an environment to test exporting, rather than
  providing multiple examples.
@Seefooo
Cleanup README
0572e16
@Seefooo
Merge branch 'main' into update_prom_rw_exporter
6a62431
@srikanthccv
Copy link
Member

Thanks!

@srikanthccv srikanthccv merged commit 0dc16a4 into open-telemetry:main Oct 31, 2022
saartochner-lumigo pushed a commit to lumigo-io/opentelemetry-python-contrib that referenced this pull request Nov 13, 2022
@Seefooo
Update prom rw exporter (open-telemetry#1359)
d0a3fe5
saartochner-lumigo pushed a commit to lumigo-io/opentelemetry-python-contrib that referenced this pull request Nov 13, 2022
@Seefooo
Update prom rw exporter (open-telemetry#1359)
2c09a5f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aabmass aabmass aabmass approved these changes

Assignees

@srikanthccv srikanthccv

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Restore prometheus remote write exporter
4 participants
@Seefooo @srikanthccv @aabmass @ocelotl