Allow custom labels to be added to net/http metrics

[Aneurysm9]

Fixes #195

[MrAlias]

Looks good. Aside from the question on needing a mutex, nothing is blocking.

[jmacd]

I wonder if this functionality ought to be achieved using Correlation Context. The OpenCensus system had a TTL=0 concept that meant the correlations would only survive within a single process. Then we wouldn't need a new API here, I think.

[MrAlias]

I wonder if this functionality ought to be achieved using Correlation Context. The OpenCensus system had a TTL=0 concept that meant the correlations would only survive within a single process. Then we wouldn't need a new API here, I think.

That's a really interesting idea. It could make this solution more universal for all instrumentation.

[Aneurysm9]

I wonder if this functionality ought to be achieved using Correlation Context. The OpenCensus system had a TTL=0 concept that meant the correlations would only survive within a single process. Then we wouldn't need a new API here, I think.

That's a really interesting idea. It could make this solution more universal for all instrumentation.

I wonder how this would work to get data from the handler up to the instrumentation. It seems that context flow is one-way. Or are you proposing that the instrumentation add the Labeler to the Correlation Context rather than directly to the context.Context?

[MrAlias]

I wonder how this would work to get data from the handler up to the instrumentation. It seems that context flow is one-way. Or are you proposing that the instrumentation add the Labeler to the Correlation Context rather than directly to the context.Context?

Good point. I think I was envisioning the use to add context prior to it getting to the middleware of the handler, but I don't think that is going to be easily achieved nor desirable to a user. It seems like they would rather an explicit solution like what you have here.

[jmacd]

I reviewed this PR and thought about the underlying request.
This feature offers an equivalent to the act of calling Span.SetAttribute() during a request, with the intent of adding labels to metrics associated with the effective Span. This is one reason people like to (at least) think about automatically converting spans to metrics--because we already have APIs to modify the current Span, therefore we wouldn't need new APIs to support adding labels to correlated metrics mid-span. My proposal about Resource scope was intended to address a similar problem: open-telemetry/oteps#78. That proposal stops short of adding a way to modify a current scope (equivalent to SetAttribute), it only gave us a way to create new scopes, but the same idea could be extended to support mutable scopes.

The use of Context in this PR, at a first glance, leads to confusion. We're only using context to pass a variable from the server to the handler. We would not expect the same labeler to be used in a child, that's why I prefer to think of this as "modify the current metric scope".

After studying this, Correlation Context is not a good way to propagate these labels--it would lead to an awkward API. Short of something radical like OTEP issue 78 (Resource scope), I don't see a way to avoid an interface like Labeler in this PR.

[derekperkins]

That's awesome, thanks for pushing that through! My only question on the PR is whether or not the labeler should be pulled out so that it can be shared between other implementations like grpc.