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

feat(otel): add ability to setup and create metrics, initially for Prometheus #45

Closed
wants to merge 1 commit into from

Conversation

BradleyChatha
Copy link
Contributor

@BradleyChatha BradleyChatha commented Jul 18, 2024

Context

This is part of the work for adding custom metrics into our applications, and I'm not super sure on the best approach especially when it comes to gokit, so this draft PR is here to serve as a testing ground on how we want this all to really work.

I've been battling on the best way to really deal with this, as SetupGlobalState is nautrally already hard to test properly, however making it as easy as possible to begin adding metrics - primarily around ease of use for setting up the Prometheus endpoint - I decided to stuff a bit more logic into the function... as bloated as it currently is.

I decided that since otelkit already sets up most of what we need for OTEL, it'd be the best place to stuff the metrics stuff into it - though that now makes the /tracing/ part of its path slightly misleading 😓

I was also struggling to determine between either:

  • Make OTEL upload to cloud monitoring directly.
  • Use Prometheus so we can setup Managed Prometheus to scrape the metrics.

In the end I felt that setting up a Prometheus endpoint would give us a bit more flexibilty, as we can manage aspects such as scrape interval; filtering out boring metrics we don't want to pay for, etc. within Managed Prometheus rather than the application's code/config.

Prometheus also makes it easier for us to inspect metrics locally, as we don't need to make local dev apps send things into Cloud Monitoring (conflicting with other people's local deployments and any live dev deployments).

Since we use GRPC for the most part I decided to add WithPrometheusMetricExporterAutoServer that makes it simple to add in the Prometheus endpoint, but this resulted in some additional complexity within otelkit's side. I'm not really sold on this idea, but wanted to pitch it.

(Minor point right now but Managed Prometheus is cheaper than direct API calls as well).

This PR

This PR:

  • Extends SetupGlobalState so that it will also configured a MeterProvider (defaulting to a noop one) alongside all the other bits of OTEL state we setup there.
  • Extends SetupGlobalState so that it'll run a deferred list of functions at the end of its logic. The main use case right now is so that WithPrometheusMetricExporterAutoServer can setup it's simple HTTP server.
  • Adds WithPrometheusMetricExporter and WithPrometheusMetricExporterAutoServer which will configure OTEL to use Prometheus for its metrics backend.

## What I'm looking for

I'm mainly looking for general feedback on whether this is the kind of approach we'd like to go down and explore, and if so I can pick a service to do a test implementation of to make sure we're all happy with this approach. The setup of otelkit is kind of secondary I guess.

As with tracing this is mainly just a light wrapper around setting up OTEL in general. When it comes to producing metrics it'll generally look like normal OTEL SDK stuff (except we use our own .Meter function).

I should note that the GCP SDKs seem to automatically use OTEL for generating metrics, so it's not just our own custom ones we have a chance to export here.

@BradleyChatha
Copy link
Contributor Author

I don't like this particular approach anymore, so I'm closing this PR for a bit until I/someone else revisits the idea.

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.

1 participant