Skip to content

Combine the Push and Pull metric controllers#1378

Merged
Aneurysm9 merged 45 commits intoopen-telemetry:masterfrom
jmacd:jmacd/pushpull
Jan 13, 2021
Merged

Combine the Push and Pull metric controllers#1378
Aneurysm9 merged 45 commits intoopen-telemetry:masterfrom
jmacd:jmacd/pushpull

Conversation

@jmacd
Copy link
Copy Markdown
Contributor

@jmacd jmacd commented Nov 30, 2020

This combines the two metric controllers due to (1) substantial overlap and (2) a real use documented in open-telemetry/opentelemetry-specification#1207 (@sagikazarmark). This adds a new example demonstrating how to export to both Prometheus (Pull) and OTLP (Push).

To combine these features, the controller's Start() method becomes optional, since starting the controller is not needed when using manual Collect() calls (a Pull-Only) configuration.

Although the diff looks quite large, this is not much more than a simple combination of the code. In controller.go the structure is essentially the former push.go with two methods from pull.go added (Collect() and ForEach()).

Support for export and collection timeouts is introduced & tested. This closes an old TODO about where the context for asynchronous instruments comes from (with tests).

Fixes #1349.
Fixes #998.

@jmacd jmacd added the area:metrics Part of OpenTelemetry Metrics label Nov 30, 2020
@jmacd
Copy link
Copy Markdown
Contributor Author

jmacd commented Nov 30, 2020

Reviewers, I'll try to modify this PR so that the diffs show more clearly.

Comment thread sdk/metric/controller/basic/controller.go
Comment thread example/prom-collector/go.mod Outdated
@hstan hstan mentioned this pull request Dec 18, 2020
@jmacd
Copy link
Copy Markdown
Contributor Author

jmacd commented Jan 5, 2021

@bogdandrutu Please review.

Copy link
Copy Markdown
Contributor

@dashpole dashpole 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 overall

Comment thread CHANGELOG.md Outdated
Comment thread exporters/metric/prometheus/prometheus.go Outdated
Comment thread example/prom-collector/main.go Outdated
@jmacd
Copy link
Copy Markdown
Contributor Author

jmacd commented Jan 13, 2021

I propose we merge this.

@Aneurysm9 Aneurysm9 merged commit dfece3d into open-telemetry:master Jan 13, 2021
@jmacd jmacd deleted the jmacd/pushpull branch February 4, 2021 18:36
@pellared pellared added this to the untracked milestone Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:metrics Part of OpenTelemetry Metrics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ability to flush metrics on demand from push controller Push controller allowed to start multiple times

7 participants