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

Make it possible to support runtime metrics on a seperate path #173

Conversation

hsyed-dojo
Copy link
Contributor

@hsyed-dojo hsyed-dojo commented Nov 15, 2022

@SuperQ

This PR introduces a new flag to make it possible to serve the go runtime metrics on a seperate endpoint. The way we are orchestrating the deployment of the exporter (multiple k8s deployments), this flag will effectively halve our stackdriver API calls.

Manual testing done:

  • With and without flag.
  • Launch process locally with one of our GCP projects.
  • Verify endpoint(s) contain what they should.

Signed-off-by: Hassan Syed <hassan.syed@paymentsense.com>
@hsyed-dojo hsyed-dojo force-pushed the hsyed/support-runtime-metrics-on-seperate-path branch from 0b2c549 to d6dd618 Compare November 15, 2022 12:19
@hsyed-dojo hsyed-dojo marked this pull request as draft November 15, 2022 12:24
Signed-off-by: Hassan Syed <hassan.syed@paymentsense.com>
@hsyed-dojo hsyed-dojo force-pushed the hsyed/support-runtime-metrics-on-seperate-path branch from ebaaeec to 80c41e2 Compare November 15, 2022 12:26
@hsyed-dojo hsyed-dojo marked this pull request as ready for review November 15, 2022 12:30
@SuperQ
Copy link
Contributor

SuperQ commented Nov 15, 2022

Interesting idea. What specifically about your deployments will reduce the number of calls?

@hsyed-dojo
Copy link
Contributor Author

hsyed-dojo commented Nov 15, 2022

Interesting idea. What specifically about your deployments will reduce the number of calls?

@SuperQ we intend to orchestrate our scrapers to have a seperate scrape job for the runtime metrics -- possibly with a tighter interval.

As we have to orchestrate with multiple deployments due to quantity of metrics we need to ingest (divide and conquer) , this approach has the additional benefit of allowing us to model multiple stackdriver_exporter deployments ergonomically in our helm meta-chart.

@SuperQ
Copy link
Contributor

SuperQ commented Nov 15, 2022

I wonder if we should have this flag do the opposite. Have web.telemetry-path be where the runtime metrics live. But move allow moving the stackdriver metrics to a new path via a separate registry. For example --web.stackdriver-telemetry-path=/stackdriver.

I'm suggesting this because we're likely to move the web.telemetry-path feature to the exporter-toolkit. It would be easier to move the exported metrics, rather than the standard telemetry metrics. Similar to how some exporters have /scrape endpoints for the multi-target exporter pattern.

@hsyed-dojo
Copy link
Contributor Author

@SuperQ yes changing the polarity of the flag would be the same for us. I'll update the PR.


We explored using query params to provide the filters and this might allow us to have a single deployment. The stackdriver API call metrics are specific to the registry created for the exposition and they reset on each call with the approach (IIRC). I think we would want those metrics to probably be cumulative/global (or with a use-case label --e.g., pubsub) and available with the runtime metrics. I hope this will be possible in the future.

Signed-off-by: Hassan Syed <hassan.syed@paymentsense.com>
Copy link
Contributor

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Nice, thanks.

@SuperQ SuperQ merged commit 33894dc into prometheus-community:master Nov 15, 2022
@hsyed-dojo hsyed-dojo deleted the hsyed/support-runtime-metrics-on-seperate-path branch November 15, 2022 16:39
@kgeckhart kgeckhart mentioned this pull request Jan 25, 2023
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