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

Add metrics for otelhttp client #427

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion go.sum
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
vchitai marked this conversation as resolved.
Show resolved Hide resolved
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand All @@ -10,14 +11,14 @@ github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE=
github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0 h1:4G4v2dO3VZwixGIRoQ5Lfboy6nUhCyYzaqnIAPPhYs4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd0=
github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
go.opentelemetry.io/otel v0.13.0 h1:2isEnyzjjJZq6r2EKMsFj4TxiQiexsM04AVhwbR/oBA=
go.opentelemetry.io/otel v0.13.0/go.mod h1:dlSNewoRYikTkotEnxdmuBHgzT+k/idJSfDv/FxEnOY=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 h1:E7g+9GITq07hpfrRu66IVDexMakfv52eLZ2CXBWiKr4=
golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 h1:YR8cESwS4TdDjEe65xsg0ogRM/Nc3DYOhEAlW+xobZo=
gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
5 changes: 5 additions & 0 deletions instrumentation/net/http/otelhttp/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ const (
ServerLatency = "http.server.duration" // Incoming end to end duration, microseconds
)

// Client HTTP metrics
vchitai marked this conversation as resolved.
Show resolved Hide resolved
const (
ClientRequestDuration = "http.client.duration" // Incoming end to end duration, microseconds
vchitai marked this conversation as resolved.
Show resolved Hide resolved
)

// Filter is a predicate used to determine whether a given http.request should
// be traced. A Filter must return true if the request should be traced.
type Filter func(*http.Request) bool
206 changes: 206 additions & 0 deletions instrumentation/net/http/otelhttp/stats.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
// Copyright The OpenTelemetry Authors
vchitai marked this conversation as resolved.
Show resolved Hide resolved
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package otelhttp

import (
"context"
"io"
"net/http"
"sync"
"time"

"go.opentelemetry.io/otel/unit"

"go.opentelemetry.io/otel/api/metric"
"go.opentelemetry.io/otel/label"
)

type statTransport struct {
meter metric.Meter
base *Transport
valueRecorders map[string]metric.Float64ValueRecorder
vchitai marked this conversation as resolved.
Show resolved Hide resolved
}

type tracker struct {
ctx context.Context
start time.Time
body io.ReadCloser
statusCode int
endOnce sync.Once
labels *label.Set

valueRecorders map[string]metric.Float64ValueRecorder
}

// The following tags are applied to stats recorded by this package. Host, Path
// and Method are applied to all measures. StatusCode is not applied to
// ClientRequestCount or ServerRequestCount, since it is recorded before the status is known.
var (
// Method is the HTTP method of the request, capitalized (GET, POST, etc.).
Method = label.Key("http.method")
// Host is the value of the HTTP Host header.
//
// The value of this tag can be controlled by the HTTP client, so you need
// to watch out for potentially generating high-cardinality labels in your
// metrics backend if you use this tag in views.
Host = label.Key("http.host")

// Scheme is the URI scheme identifying the used protocol: "http" or "https"
Scheme = label.Key("http.scheme")

// StatusCode is the numeric HTTP response status code,
// or "error" if a transport error occurred and no status code was read.
StatusCode = label.Key("http.status")

// Path is the URL path (not including query string) in the request.
//
// The value of this tag can be controlled by the HTTP client, so you need
// to watch out for potentially generating high-cardinality labels in your
// metrics backend if you use this tag in views.
Path = label.Key("http.path")
)
vchitai marked this conversation as resolved.
Show resolved Hide resolved

// Client tag keys.
var (
// KeyClientMethod is the HTTP method, capitalized (i.e. GET, POST, PUT, DELETE, etc.).
KeyClientMethod = label.Key("http_client_method")
// KeyClientPath is the URL path (not including query string).
KeyClientPath = label.Key("http_client_path")
// KeyClientStatus is the HTTP status code as an integer (e.g. 200, 404, 500.), or "error" if no response status line was received.
KeyClientStatus = label.Key("http_client_status")
// KeyClientHost is the value of the request Host header.
KeyClientHost = label.Key("http_client_host")
// KeyClientScheme is the URI scheme identifying the used protocol: "http" or "https"
KeyClientScheme = label.Key("http_client_host")
)

func (trans *statTransport) applyConfig(c *config) {
trans.base.applyConfig(c)

trans.meter = c.Meter
trans.createMeasures()
}

// RoundTrip implements http.RoundTripper, delegating to Base and recording stats for the request.
func (trans *statTransport) RoundTrip(req *http.Request) (*http.Response, error) {
labels := label.NewSet(
// conform open-telemetry label definition
Method.String(req.Method),
Host.String(req.Host),
Scheme.String(req.URL.Scheme),
Path.String(req.URL.Path),
// for prometheus
KeyClientMethod.String(req.Method),
KeyClientHost.String(req.Host),
KeyClientScheme.String(req.URL.Scheme),
KeyClientPath.String(req.URL.Path),
vchitai marked this conversation as resolved.
Show resolved Hide resolved
)

ctx := req.Context()
track := &tracker{
start: time.Now(),
ctx: ctx,
valueRecorders: trans.valueRecorders,
labels: &labels,
}

// Perform request.
resp, err := trans.base.RoundTrip(req)
if err != nil {
track.statusCode = http.StatusInternalServerError
track.end()
} else {
track.statusCode = resp.StatusCode
if resp.Body == nil {
track.end()
} else {
track.body = resp.Body
resp.Body = wrappedBodyIO(track, resp.Body)
Copy link
Contributor

@MrAlias MrAlias Nov 5, 2020

Choose a reason for hiding this comment

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

Reading of the response body should not be included in the measurement of a requests duration. Right?

Copy link
Author

@vchitai vchitai Nov 13, 2020

Choose a reason for hiding this comment

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

The request will be counted as ended after we finish reading all the data from the response body, won't it?

Copy link
Member

Choose a reason for hiding this comment

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

There is probably utility in measurements of both time to first byte and time to the end of the body being read, though the latter is potentially less a measure of the client and more of whatever is processing the response data.

Copy link
Author

Choose a reason for hiding this comment

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

This only wrap and returns reader interface rather than actual read and parsing actual data itself. The actual parsing time could be useful, but should not be included in this package, dont you think ?

}
}
return resp, err
}

// wrappedBodyIO returns a wrapped version of the original
// Body and only implements the same combination of additional
// interfaces as the original.
func wrappedBodyIO(wrapper io.ReadCloser, body io.ReadCloser) io.ReadCloser {
wr, i0 := body.(io.Writer)
switch {
case !i0:
return struct {
io.ReadCloser
}{wrapper}

case i0:
return struct {
io.ReadCloser
io.Writer
}{wrapper, wr}
default:
return struct {
io.ReadCloser
}{wrapper}
}
}

func (trans *statTransport) createMeasures() {
trans.valueRecorders = make(map[string]metric.Float64ValueRecorder)

requestDurationMeasure, err := trans.meter.NewFloat64ValueRecorder(
ClientRequestDuration,
metric.WithDescription("measure the duration of the outbound HTTP request"),
metric.WithUnit(unit.Milliseconds),
)
handleErr(err)

trans.valueRecorders[ClientRequestDuration] = requestDurationMeasure
}

var _ io.ReadCloser = (*tracker)(nil)

func (t *tracker) end() {
t.endOnce.Do(func() {
latencyMs := float64(time.Since(t.start)) / float64(time.Millisecond)
labels := label.NewSet(
append(t.labels.ToSlice(),
StatusCode.Int(t.statusCode),
KeyClientStatus.Int(t.statusCode),
)...,
)
ls := labels.ToSlice()

t.valueRecorders[ClientRequestDuration].Record(t.ctx, latencyMs, ls...)
})
}

func (t *tracker) Read(b []byte) (int, error) {
n, err := t.body.Read(b)
switch err {
case nil:
return n, nil
case io.EOF:
t.end()
}
return n, err
}

func (t *tracker) Close() error {
// Invoking endSpan on Close will help catch the cases
// in which a read returned a non-nil error, we set the
// span status but didn't end the span.
t.end()
return t.body.Close()
}
8 changes: 5 additions & 3 deletions instrumentation/net/http/otelhttp/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ var _ http.RoundTripper = &Transport{}

// NewTransport wraps the provided http.RoundTripper with one that
// starts a span and injects the span context into the outbound request headers.
func NewTransport(base http.RoundTripper, opts ...Option) *Transport {
t := Transport{
rt: base,
func NewTransport(base http.RoundTripper, opts ...Option) http.RoundTripper {
Copy link
Member

Choose a reason for hiding this comment

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

Since this is the primary mechanism by which I expect otelhttp.Transport instances to be created and this leaves no configuration option for enabling/disabling metric collection, does it make more sense to integrate the metrics collection directly in the Transport.RoundTrip method? This would also be more in line with the approach taken by the otelhttp.Handler.

Copy link
Author

Choose a reason for hiding this comment

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

I prefer to split up the implementation of metrics and tracing parts for ease of maintenance.

t := statTransport{
base: &Transport{
rt: base,
},
}

defaultOpts := []Option{
Expand Down