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

feature: exemplar support for promhttp #854

Closed
bwplotka opened this issue Apr 10, 2021 · 6 comments · Fixed by #1055
Closed

feature: exemplar support for promhttp #854

bwplotka opened this issue Apr 10, 2021 · 6 comments · Fixed by #1055

Comments

@bwplotka
Copy link
Member

bwplotka commented Apr 10, 2021

Problem Statement

I am building HTTP client and server instrumentation for both metrics and tracing, but it's currently hard/impossible to compose both using promhttp

This will become more and more "normal" so wonder if there a good way to extend promhttp to Instrument with Exermplars. Right now I have to do things on my own:

	return promhttp.InstrumentRoundTripperInFlight(
		ins.requestsInFlight.WithLabelValues("target"),
		// TODO(bwplotka): Can't use promhttp.InstrumentRoundTripperCounter or  promhttp.InstrumentRoundTripperDuration, propose exemplars feature.
		promhttp.RoundTripperFunc(func(req *http.Request) (*http.Response, error) {
			now := time.Now()
			resp, err := next.RoundTrip(req)
			if err != nil {
				return resp, err
			}

			cntr := ins.requestsTotal.WithLabelValues(targetName, strings.ToLower(req.Method), fmt.Sprintf("%d", resp.StatusCode))
			observer := ins.requestDuration.WithLabelValues(targetName, strings.ToLower(req.Method), fmt.Sprintf("%d", resp.StatusCode))
			// If we find a TraceID from OpenTelemetry we'll expose it as Exemplar.
			if spanCtx := trace.SpanContextFromContext(req.Context()); spanCtx.HasTraceID() {
				traceID := prometheus.Labels{"traceID": spanCtx.TraceID().String()}

				cntr.(prometheus.ExemplarAdder).AddWithExemplar(1, traceID)
				observer.(prometheus.ExemplarObserver).ObserveWithExemplar(time.Since(now).Seconds(), traceID)
				return resp, err
			}

			cntr.Inc()
			observer.Observe(time.Since(now).Seconds())
			return resp, err
		}),
	)

Proposal

Maybe something similar to promauto.With?


promhttp.WithExemplarFunc(fn func(ctx context.Context) prometheus.Labels).InstrumentRoundTripperCounter(....)

WDYT? @beorn7 (:

@bwplotka
Copy link
Member Author

bwplotka commented Apr 10, 2021

Actually better idea would be to add

type ObserverVec interface {
	....
	CurryWithExemplar(Labels) ObserverVec
}

(and same for counter and gauge)

That would solve most of the cases (:

@beorn7
Copy link
Member

beorn7 commented Apr 10, 2021

Would be nice to have an easy way using exemplars with the instrumentation tools in promhttp. But I'm not sure how your 2nd idea would help. How do you inject a trace ID in that case? I guess the user needs to provide a function to dynamically determine the labels (like a trace ID) and perhaps also control sampling (so that you could decide to add an exemplar only every 10th request or for "interesting" requests etc.).

@bwplotka
Copy link
Member Author

You are right 2nd idea is bad, I forgot you need a dynamic thing and the following might be too crazy:

CurryWithExemplarFn(fn func(ctx context.Context) prometheus.Labels) ObserverVec

guess the user needs to provide a function to dynamically determine the labels (like a trace ID) and perhaps also control sampling (so that you could decide to add an exemplar only every 10th request or for "interesting" requests etc.).

I thought about it, and I don't think sampling is needed 🤔 If you choose to have exemplars, it might be ok to add all of them on each observation/increment. User can choose which metric to have with exemplar, but once they do they won't be able to choose per sampling/code/method. I wonder if it's fine at the end. Why not just adding all of codes/methods?

@beorn7
Copy link
Member

beorn7 commented Apr 12, 2021

I'm thinking about high-frequency observations. The Observe call is designed to be as fast as possible while staying concurrency-safe. Calling ObserveWithExemplar is already much more work than just calling Observe (I'd guess most of it due to allocating the Exemplar DTO). And then there is possible work on the caller's side, like retrieving the trace ID.

The proposed function could just have the convention that returning an empty label set means that this observation should be done without an exemplar. In that way, the caller can implement sampling if they need.

Different thing: I wouldn't use the word “Curry” here. It has a rather specific meaning. Using it for partially setting labels as already a bit of a stretch, but the similarity is still visible (IMHO). Adding an exemplar to an observation is just too different.

@just1900
Copy link

just1900 commented Dec 2, 2021

Sorry to bother, any updates on this?
It seems client_java have supported exemplars with otel.

@bwplotka
Copy link
Member Author

#1055

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants