Fix IdP client metrics (#2810)

This commit is contained in:
Travis Groth 2021-12-08 13:22:53 -05:00 committed by GitHub
parent d0890d399c
commit e2e0646f70
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 32 additions and 31 deletions

View file

@ -57,7 +57,7 @@ func WithLoginURL(loginURL *url.URL) Option {
// WithHTTPClient sets the http client to use for requests to the Azure APIs. // WithHTTPClient sets the http client to use for requests to the Azure APIs.
func WithHTTPClient(httpClient *http.Client) Option { func WithHTTPClient(httpClient *http.Client) Option {
return func(cfg *config) { return func(cfg *config) {
cfg.httpClient = httputil.NewLoggingClient(httpClient, cfg.httpClient = httputil.NewLoggingClient(httpClient, "azure_idp_client",
func(evt *zerolog.Event) *zerolog.Event { func(evt *zerolog.Event) *zerolog.Event {
return evt.Str("provider", "azure") return evt.Str("provider", "azure")
}) })

View file

@ -46,7 +46,7 @@ func WithServiceAccount(serviceAccount *ServiceAccount) Option {
// WithHTTPClient sets the http client option. // WithHTTPClient sets the http client option.
func WithHTTPClient(httpClient *http.Client) Option { func WithHTTPClient(httpClient *http.Client) Option {
return func(cfg *config) { return func(cfg *config) {
cfg.httpClient = httputil.NewLoggingClient(httpClient, cfg.httpClient = httputil.NewLoggingClient(httpClient, "github_idp_client",
func(evt *zerolog.Event) *zerolog.Event { func(evt *zerolog.Event) *zerolog.Event {
return evt.Str("provider", "github") return evt.Str("provider", "github")
}) })

View file

@ -46,7 +46,7 @@ func WithServiceAccount(serviceAccount *ServiceAccount) Option {
// WithHTTPClient sets the http client option. // WithHTTPClient sets the http client option.
func WithHTTPClient(httpClient *http.Client) Option { func WithHTTPClient(httpClient *http.Client) Option {
return func(cfg *config) { return func(cfg *config) {
cfg.httpClient = httputil.NewLoggingClient(httpClient, cfg.httpClient = httputil.NewLoggingClient(httpClient, "gitlab_idp_client",
func(evt *zerolog.Event) *zerolog.Event { func(evt *zerolog.Event) *zerolog.Event {
return evt.Str("provider", "azure") return evt.Str("provider", "azure")
}) })

View file

@ -62,7 +62,7 @@ func WithBatchSize(batchSize int) Option {
// WithHTTPClient sets the http client option. // WithHTTPClient sets the http client option.
func WithHTTPClient(httpClient *http.Client) Option { func WithHTTPClient(httpClient *http.Client) Option {
return func(cfg *config) { return func(cfg *config) {
cfg.httpClient = httputil.NewLoggingClient(httpClient, cfg.httpClient = httputil.NewLoggingClient(httpClient, "okta_idp_client",
func(evt *zerolog.Event) *zerolog.Event { func(evt *zerolog.Event) *zerolog.Event {
return evt.Str("provider", "okta") return evt.Str("provider", "okta")
}) })

View file

@ -44,7 +44,7 @@ func WithBatchSize(batchSize int) Option {
// WithHTTPClient sets the http client option. // WithHTTPClient sets the http client option.
func WithHTTPClient(httpClient *http.Client) Option { func WithHTTPClient(httpClient *http.Client) Option {
return func(cfg *config) { return func(cfg *config) {
cfg.httpClient = httputil.NewLoggingClient(httpClient, cfg.httpClient = httputil.NewLoggingClient(httpClient, "onelogin_idp_client",
func(evt *zerolog.Event) *zerolog.Event { func(evt *zerolog.Event) *zerolog.Event {
return evt.Str("provider", "onelogin") return evt.Str("provider", "onelogin")
}) })

View file

@ -47,7 +47,7 @@ func WithEnvironmentID(environmentID string) Option {
// WithHTTPClient sets the http client option. // WithHTTPClient sets the http client option.
func WithHTTPClient(httpClient *http.Client) Option { func WithHTTPClient(httpClient *http.Client) Option {
return func(cfg *config) { return func(cfg *config) {
cfg.httpClient = httputil.NewLoggingClient(httpClient, cfg.httpClient = httputil.NewLoggingClient(httpClient, "ping_idp_client",
func(evt *zerolog.Event) *zerolog.Event { func(evt *zerolog.Event) *zerolog.Event {
return evt.Str("provider", "ping") return evt.Str("provider", "ping")
}) })

View file

@ -57,13 +57,16 @@ func NewLoggingRoundTripper(base http.RoundTripper, customize ...func(event *zer
} }
// NewLoggingClient creates a new http.Client that will log requests. // NewLoggingClient creates a new http.Client that will log requests.
func NewLoggingClient(base *http.Client, customize ...func(event *zerolog.Event) *zerolog.Event) *http.Client { func NewLoggingClient(base *http.Client, name string, customize ...func(event *zerolog.Event) *zerolog.Event) *http.Client {
if base == nil { if base == nil {
base = http.DefaultClient base = http.DefaultClient
} }
newClient := new(http.Client) newClient := new(http.Client)
*newClient = *base *newClient = *base
newClient.Transport = NewLoggingRoundTripper(newClient.Transport, customize...) newClient.Transport = tripper.NewChain(metrics.HTTPMetricsRoundTripper(func() string {
return ""
}, name)).Then(NewLoggingRoundTripper(newClient.Transport, customize...))
return newClient return newClient
} }
@ -75,7 +78,7 @@ type httpClient struct {
func (c *httpClient) Do(req *http.Request) (*http.Response, error) { func (c *httpClient) Do(req *http.Request) (*http.Response, error) {
tripperChain := tripper.NewChain(metrics.HTTPMetricsRoundTripper(func() string { tripperChain := tripper.NewChain(metrics.HTTPMetricsRoundTripper(func() string {
return "" return ""
}, "idp_http_client", req.Host)) }, "idp_http_client"))
c.Client.Transport = tripperChain.Then(c.requestIDTripper) c.Client.Transport = tripperChain.Then(c.requestIDTripper)
return c.Client.Do(req) return c.Client.Do(req)
} }

View file

@ -7,8 +7,8 @@ import (
) )
// HTTPStatsRoundTripper creates tracing and metrics RoundTripper for a pomerium service // HTTPStatsRoundTripper creates tracing and metrics RoundTripper for a pomerium service
func HTTPStatsRoundTripper(getInstallationID func() string, service string, destination string) func(next http.RoundTripper) http.RoundTripper { func HTTPStatsRoundTripper(getInstallationID func() string, service string) func(next http.RoundTripper) http.RoundTripper {
return metrics.HTTPMetricsRoundTripper(getInstallationID, ServiceName(service), destination) return metrics.HTTPMetricsRoundTripper(getInstallationID, ServiceName(service))
} }
// HTTPStatsHandler creates tracing and metrics Handler for a pomerium service // HTTPStatsHandler creates tracing and metrics Handler for a pomerium service

View file

@ -14,7 +14,6 @@ var (
TagKeyGRPCService = tag.MustNewKey("grpc_service") TagKeyGRPCService = tag.MustNewKey("grpc_service")
TagKeyGRPCMethod = tag.MustNewKey("grpc_method") TagKeyGRPCMethod = tag.MustNewKey("grpc_method")
TagKeyHost = tag.MustNewKey("host") TagKeyHost = tag.MustNewKey("host")
TagKeyDestination = tag.MustNewKey("destination")
TagKeyStorageOperation = tag.MustNewKey("operation") TagKeyStorageOperation = tag.MustNewKey("operation")
TagKeyStorageResult = tag.MustNewKey("result") TagKeyStorageResult = tag.MustNewKey("result")

View file

@ -74,7 +74,7 @@ var (
Name: "http/client/requests_total", Name: "http/client/requests_total",
Measure: ochttp.ClientRoundtripLatency, Measure: ochttp.ClientRoundtripLatency,
Description: "Total HTTP Client Requests", Description: "Total HTTP Client Requests",
TagKeys: []tag.Key{TagKeyService, TagKeyHost, TagKeyHTTPMethod, ochttp.StatusCode, TagKeyDestination}, TagKeys: []tag.Key{TagKeyService, TagKeyHost, TagKeyHTTPMethod, ochttp.StatusCode},
Aggregation: view.Count(), Aggregation: view.Count(),
} }
@ -84,7 +84,7 @@ var (
Name: "http/client/request_duration_ms", Name: "http/client/request_duration_ms",
Measure: ochttp.ClientRoundtripLatency, Measure: ochttp.ClientRoundtripLatency,
Description: "HTTP Client Request duration in ms", Description: "HTTP Client Request duration in ms",
TagKeys: []tag.Key{TagKeyService, TagKeyHost, TagKeyHTTPMethod, ochttp.StatusCode, TagKeyDestination}, TagKeys: []tag.Key{TagKeyService, TagKeyHost, TagKeyHTTPMethod, ochttp.StatusCode},
Aggregation: DefaultHTTPLatencyDistrubtion, Aggregation: DefaultHTTPLatencyDistrubtion,
} }
@ -94,7 +94,7 @@ var (
Name: "http/client/response_size_bytes", Name: "http/client/response_size_bytes",
Measure: ochttp.ClientReceivedBytes, Measure: ochttp.ClientReceivedBytes,
Description: "HTTP Client Response Size in bytes", Description: "HTTP Client Response Size in bytes",
TagKeys: []tag.Key{TagKeyService, TagKeyHost, TagKeyHTTPMethod, ochttp.StatusCode, TagKeyDestination}, TagKeys: []tag.Key{TagKeyService, TagKeyHost, TagKeyHTTPMethod, ochttp.StatusCode},
Aggregation: DefaulHTTPSizeDistribution, Aggregation: DefaulHTTPSizeDistribution,
} }
@ -104,7 +104,7 @@ var (
Name: "http/client/response_size_bytes", Name: "http/client/response_size_bytes",
Measure: ochttp.ClientSentBytes, Measure: ochttp.ClientSentBytes,
Description: "HTTP Client Response Size in bytes", Description: "HTTP Client Response Size in bytes",
TagKeys: []tag.Key{TagKeyService, TagKeyHost, TagKeyHTTPMethod, TagKeyDestination}, TagKeys: []tag.Key{TagKeyService, TagKeyHost, TagKeyHTTPMethod},
Aggregation: DefaulHTTPSizeDistribution, Aggregation: DefaulHTTPSizeDistribution,
} }
) )
@ -137,7 +137,7 @@ func HTTPMetricsHandler(getInstallationID func() string, service string) func(ne
} }
// HTTPMetricsRoundTripper creates a metrics tracking tripper for outbound HTTP Requests // HTTPMetricsRoundTripper creates a metrics tracking tripper for outbound HTTP Requests
func HTTPMetricsRoundTripper(getInstallationID func() string, service string, destination string) func(next http.RoundTripper) http.RoundTripper { func HTTPMetricsRoundTripper(getInstallationID func() string, service string) func(next http.RoundTripper) http.RoundTripper {
return func(next http.RoundTripper) http.RoundTripper { return func(next http.RoundTripper) http.RoundTripper {
return tripper.RoundTripperFunc(func(r *http.Request) (*http.Response, error) { return tripper.RoundTripperFunc(func(r *http.Request) (*http.Response, error) {
ctx, tagErr := tag.New( ctx, tagErr := tag.New(
@ -145,7 +145,6 @@ func HTTPMetricsRoundTripper(getInstallationID func() string, service string, de
tag.Upsert(TagKeyService, service), tag.Upsert(TagKeyService, service),
tag.Upsert(TagKeyHost, r.Host), tag.Upsert(TagKeyHost, r.Host),
tag.Upsert(TagKeyHTTPMethod, r.Method), tag.Upsert(TagKeyHTTPMethod, r.Method),
tag.Upsert(TagKeyDestination, destination),
) )
if tagErr != nil { if tagErr != nil {
log.Warn(ctx).Err(tagErr).Str("context", "HTTPMetricsRoundTripper").Msg("telemetry/metrics: failed to create metrics tag") log.Warn(ctx).Err(tagErr).Str("context", "HTTPMetricsRoundTripper").Msg("telemetry/metrics: failed to create metrics tag")

View file

@ -129,7 +129,7 @@ func newFailingTestTransport() http.RoundTripper {
} }
func Test_HTTPMetricsRoundTripper(t *testing.T) { func Test_HTTPMetricsRoundTripper(t *testing.T) {
chain := tripper.NewChain(HTTPMetricsRoundTripper(func() string { return "test_installation_id" }, "test_service", "test_destination")) chain := tripper.NewChain(HTTPMetricsRoundTripper(func() string { return "test_installation_id" }, "test_service"))
rt := chain.Then(newTestTransport()) rt := chain.Then(newTestTransport())
client := http.Client{Transport: rt} client := http.Client{Transport: rt}
@ -146,28 +146,28 @@ func Test_HTTPMetricsRoundTripper(t *testing.T) {
name: "good get", name: "good get",
url: "http://test.local/good", url: "http://test.local/good",
verb: "GET", verb: "GET",
wanthttpClientRequestSize: "{ { {destination test_destination}{host test.local}{http.status 200}{http_method GET}{service test_service} }&{1 5 5 5 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]", wanthttpClientRequestSize: "{ { {host test.local}{http.status 200}{http_method GET}{service test_service} }&{1 5 5 5 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]",
wanthttpClientResponseSize: "{ { {destination test_destination}{host test.local}{http.status 200}{http_method GET}{service test_service} }&{1 5 5 5 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]", wanthttpClientResponseSize: "{ { {host test.local}{http.status 200}{http_method GET}{service test_service} }&{1 5 5 5 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]",
wanthttpClientRequestDuration: "{ { {destination test_destination}{host test.local}{http.status 200}{http_method GET}{service test_service} }", wanthttpClientRequestDuration: "{ { {host test.local}{http.status 200}{http_method GET}{service test_service} }",
wanthttpClientRequestCount: "{ { {destination test_destination}{host test.local}{http.status 200}{http_method GET}{service test_service} }", wanthttpClientRequestCount: "{ { {host test.local}{http.status 200}{http_method GET}{service test_service} }",
}, },
{ {
name: "good post", name: "good post",
url: "http://test.local/good", url: "http://test.local/good",
verb: "POST", verb: "POST",
wanthttpClientRequestSize: "{ { {destination test_destination}{host test.local}{http.status 200}{http_method POST}{service test_service} }&{1 5 5 5 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]", wanthttpClientRequestSize: "{ { {host test.local}{http.status 200}{http_method POST}{service test_service} }&{1 5 5 5 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]",
wanthttpClientResponseSize: "{ { {destination test_destination}{host test.local}{http.status 200}{http_method POST}{service test_service} }&{1 5 5 5 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]", wanthttpClientResponseSize: "{ { {host test.local}{http.status 200}{http_method POST}{service test_service} }&{1 5 5 5 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]",
wanthttpClientRequestDuration: "{ { {destination test_destination}{host test.local}{http.status 200}{http_method POST}{service test_service} }", wanthttpClientRequestDuration: "{ { {host test.local}{http.status 200}{http_method POST}{service test_service} }",
wanthttpClientRequestCount: "{ { {destination test_destination}{host test.local}{http.status 200}{http_method POST}{service test_service} }", wanthttpClientRequestCount: "{ { {host test.local}{http.status 200}{http_method POST}{service test_service} }",
}, },
{ {
name: "bad post", name: "bad post",
url: "http://test.local/bad", url: "http://test.local/bad",
verb: "POST", verb: "POST",
wanthttpClientRequestSize: "{ { {destination test_destination}{host test.local}{http.status 404}{http_method POST}{service test_service} }&{1 19 19 19 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]", wanthttpClientRequestSize: "{ { {host test.local}{http.status 404}{http_method POST}{service test_service} }&{1 19 19 19 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]",
wanthttpClientResponseSize: "{ { {destination test_destination}{host test.local}{http.status 404}{http_method POST}{service test_service} }&{1 19 19 19 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]", wanthttpClientResponseSize: "{ { {host test.local}{http.status 404}{http_method POST}{service test_service} }&{1 19 19 19 0 [0 1 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0]",
wanthttpClientRequestDuration: "{ { {destination test_destination}{host test.local}{http.status 404}{http_method POST}{service test_service} }", wanthttpClientRequestDuration: "{ { {host test.local}{http.status 404}{http_method POST}{service test_service} }",
wanthttpClientRequestCount: "{ { {destination test_destination}{host test.local}{http.status 404}{http_method POST}{service test_service} }", wanthttpClientRequestCount: "{ { {host test.local}{http.status 404}{http_method POST}{service test_service} }",
}, },
} }
for _, tt := range tests { for _, tt := range tests {