Skip to content

Add requester field and enricher hook to controller metrics#58

Open
DylanBlakemore wants to merge 4 commits into
masterfrom
controller-metrics-enricher
Open

Add requester field and enricher hook to controller metrics#58
DylanBlakemore wants to merge 4 commits into
masterfrom
controller-metrics-enricher

Conversation

@DylanBlakemore
Copy link
Copy Markdown
Collaborator

Summary

  • Add requester field to Zexbox.Metrics.ControllerSeries (not populated by default)
  • Add Zexbox.Metrics.ControllerSeriesEnricher behaviour and an :enricher option on Zexbox.Metrics.start_link/1 (also configurable via config :zexbox, :metrics_enricher)
  • Invoke the enricher from the metric handler with exception fallback

Details

Host apps frequently need to attach context to the controller metric that zexbox itself doesn't know about — most immediately, an identifier for the caller derived from an API key lookup. Rather than baking specific knowledge of those values into zexbox, this adds a small extension point: a Plug-shaped behaviour (init/1 + call/3) that receives the in-flight ControllerSeries and the conn, and returns a possibly-modified series. The host app configures their enricher module either at supervision time or via application env, and uses it to copy values they've already stashed on conn.assigns (e.g. an API key description set by their auth plug) onto the series.

The new requester field is the canonical place to put that caller identifier, but the enricher is free to set any tag or field on the series. Enricher exceptions are caught by the metric handler and logged — a buggy enricher will not break telemetry; the un-enriched series is written instead.


defp enrich(series, %{conn: conn}, %{enricher: {module, opts}}) do
module.call(series, conn, opts)
rescue
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I'm in two minds about this. On one hand, it's swallowing an error instead of raising loudly. On the other hand, I'd rather not have metrics crash the process. Thoughts?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not quite sure what the best approach is either. I agree that metrics shouldn't crash a process however I also think that just swallowing errors here is opening the door to bugs slipping under the radar. If nothing else I think we should make it clear in the documentation that uncaught exceptions will be swallowed so enricher modules will need to have some kind of error handling of their own? Having said that, if we're basically telling people that they need their own rescue clauses in their enrichers is there any point in having one here?

Copy link
Copy Markdown
Collaborator Author

@DylanBlakemore DylanBlakemore May 25, 2026

Choose a reason for hiding this comment

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

@KentHawkings after some digging, I think this is necessary. An exception raised when gathering controller metrics here would detach :telemetry entirely, and metric collection would break until the app restarts. We have an outer rescue entirely, but this at least guarantees the minimal series will be written.

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