From e3af4982faf7bc3cec1f55f5a37d448583b2a007 Mon Sep 17 00:00:00 2001 From: Joseph Woodward Date: Tue, 15 Jun 2021 09:56:16 +0100 Subject: [PATCH 1/4] Initial instrumentation --- tempodb/backend/azure/azure_helpers.go | 23 ++++++++----- tempodb/backend/azure/instrumentation.go | 41 ++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 tempodb/backend/azure/instrumentation.go diff --git a/tempodb/backend/azure/azure_helpers.go b/tempodb/backend/azure/azure_helpers.go index d615d0e5da4..170a0355dd1 100644 --- a/tempodb/backend/azure/azure_helpers.go +++ b/tempodb/backend/azure/azure_helpers.go @@ -3,6 +3,7 @@ package azure import ( "context" "fmt" + "net/http" "net/url" "strings" "time" @@ -17,8 +18,8 @@ const ( uptoHedgedRequests = 2 ) -func GetContainerURL(ctx context.Context, conf *Config, hedge bool) (blob.ContainerURL, error) { - c, err := blob.NewSharedKeyCredential(conf.StorageAccountName.String(), conf.StorageAccountKey.String()) +func GetContainerURL(ctx context.Context, cfg *Config, hedge bool) (blob.ContainerURL, error) { + c, err := blob.NewSharedKeyCredential(cfg.StorageAccountName.String(), cfg.StorageAccountKey.String()) if err != nil { return blob.ContainerURL{}, err } @@ -32,10 +33,16 @@ func GetContainerURL(ctx context.Context, conf *Config, hedge bool) (blob.Contai } var httpSender pipeline.Factory - if hedge && conf.HedgeRequestsAt != 0 { + if hedge && cfg.HedgeRequestsAt != 0 { httpSender = pipeline.FactoryFunc(func(next pipeline.Policy, po *pipeline.PolicyOptions) pipeline.PolicyFunc { return func(ctx context.Context, request pipeline.Request) (pipeline.Response, error) { - client := hedgedhttp.NewClient(conf.HedgeRequestsAt, uptoHedgedRequests, nil) + + customTransport := http.DefaultTransport.(*http.Transport).Clone() + + transport := newInstrumentedTransport(customTransport) + transport = hedgedhttp.NewRoundTripper(cfg.HedgeRequestsAt, uptoHedgedRequests, transport) + + client := http.Client{Transport: transport} // Send the request over the network resp, err := client.Do(request.WithContext(ctx)) @@ -51,12 +58,12 @@ func GetContainerURL(ctx context.Context, conf *Config, hedge bool) (blob.Contai HTTPSender: httpSender, }) - u, err := url.Parse(fmt.Sprintf("https://%s.%s", conf.StorageAccountName, conf.Endpoint)) + u, err := url.Parse(fmt.Sprintf("https://%s.%s", cfg.StorageAccountName, cfg.Endpoint)) // If the endpoint doesn't start with blob.core we can assume Azurite is being used // So the endpoint should follow Azurite URL style - if !strings.HasPrefix(conf.Endpoint, "blob.core") { - u, err = url.Parse(fmt.Sprintf("http://%s/%s", conf.Endpoint, conf.StorageAccountName)) + if !strings.HasPrefix(cfg.Endpoint, "blob.core") { + u, err = url.Parse(fmt.Sprintf("http://%s/%s", cfg.Endpoint, cfg.StorageAccountName)) } if err != nil { @@ -65,7 +72,7 @@ func GetContainerURL(ctx context.Context, conf *Config, hedge bool) (blob.Contai service := blob.NewServiceURL(*u, p) - return service.NewContainerURL(conf.ContainerName), nil + return service.NewContainerURL(cfg.ContainerName), nil } func GetContainer(ctx context.Context, conf *Config, hedge bool) (blob.ContainerURL, error) { diff --git a/tempodb/backend/azure/instrumentation.go b/tempodb/backend/azure/instrumentation.go new file mode 100644 index 00000000000..f57a2ff47af --- /dev/null +++ b/tempodb/backend/azure/instrumentation.go @@ -0,0 +1,41 @@ +package azure + +import ( + "net/http" + "strconv" + "time" + + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" +) + +var ( + azureRequestDuration = promauto.NewHistogramVec(prometheus.HistogramOpts{ + Namespace: "tempodb", + Name: "azure_request_duration_seconds", + Help: "Time spent doing Azure requests.", + + Buckets: prometheus.ExponentialBuckets(0.005, 4, 6), + }, []string{"operation", "status_code"}) +) + +type instrumentedTransport struct { + observer prometheus.ObserverVec + next http.RoundTripper +} + +func newInstrumentedTransport(next http.RoundTripper) http.RoundTripper { + return instrumentedTransport{ + observer: azureRequestDuration, + next: next, + } +} + +func (i instrumentedTransport) RoundTrip(req *http.Request) (*http.Response, error) { + start := time.Now() + resp, err := i.next.RoundTrip(req) + if err == nil { + i.observer.WithLabelValues(req.Method, strconv.Itoa(resp.StatusCode)).Observe(time.Since(start).Seconds()) + } + return resp, err +} From fdbaf944226047ade966f056de40d9074777626d Mon Sep 17 00:00:00 2001 From: Joseph Woodward Date: Tue, 15 Jun 2021 22:39:57 +0100 Subject: [PATCH 2/4] Captured metrics for non hedged requests --- tempodb/backend/azure/azure_helpers.go | 28 ++++++++++++++------------ 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/tempodb/backend/azure/azure_helpers.go b/tempodb/backend/azure/azure_helpers.go index 170a0355dd1..b1daf9adf93 100644 --- a/tempodb/backend/azure/azure_helpers.go +++ b/tempodb/backend/azure/azure_helpers.go @@ -32,25 +32,27 @@ func GetContainerURL(ctx context.Context, cfg *Config, hedge bool) (blob.Contain retryOptions.TryTimeout = time.Until(deadline) } - var httpSender pipeline.Factory - if hedge && cfg.HedgeRequestsAt != 0 { - httpSender = pipeline.FactoryFunc(func(next pipeline.Policy, po *pipeline.PolicyOptions) pipeline.PolicyFunc { - return func(ctx context.Context, request pipeline.Request) (pipeline.Response, error) { + httpSender := pipeline.FactoryFunc(func(next pipeline.Policy, po *pipeline.PolicyOptions) pipeline.PolicyFunc { + return func(ctx context.Context, request pipeline.Request) (pipeline.Response, error) { - customTransport := http.DefaultTransport.(*http.Transport).Clone() + customTransport := http.DefaultTransport.(*http.Transport).Clone() - transport := newInstrumentedTransport(customTransport) + // add instrumentation + transport := newInstrumentedTransport(customTransport) + + // hedge if desired (0 means disabled) + if hedge && cfg.HedgeRequestsAt != 0 { transport = hedgedhttp.NewRoundTripper(cfg.HedgeRequestsAt, uptoHedgedRequests, transport) + } - client := http.Client{Transport: transport} + client := http.Client{Transport: transport} - // Send the request over the network - resp, err := client.Do(request.WithContext(ctx)) + // Send the request over the network + resp, err := client.Do(request.WithContext(ctx)) - return pipeline.NewHTTPResponse(resp), err - } - }) - } + return pipeline.NewHTTPResponse(resp), err + } + }) p := blob.NewPipeline(c, blob.PipelineOptions{ Retry: retryOptions, From 48d69890a5b402399b57e52ad7d5d333c9d9224d Mon Sep 17 00:00:00 2001 From: Joseph Woodward Date: Wed, 16 Jun 2021 00:11:29 +0100 Subject: [PATCH 3/4] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6a2ebea730..110e818bad2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ * [FEATURE] Added the ability to hedge requests with all backends [#750](https://github.com/grafana/tempo/pull/750) (@joe-elliott) * [ENHANCEMENT] Performance: improve compaction speed with concurrent reads and writes [#754](https://github.com/grafana/tempo/pull/754) (@mdisibio) * [ENHANCEMENT] Improve readability of cpu and memory metrics on operational dashboard [#764](https://github.com/grafana/tempo/pull/764) (@bboreham) +* [ENHANCEMENT] Add `azure_request_duration_seconds` metric. [#767](https://github.com/grafana/tempo/pull/767) (@JosephWoodward) ## v1.0.1 From 20fdb685e17f86d9faa4f0863a83f9b414757960 Mon Sep 17 00:00:00 2001 From: Joseph Woodward Date: Wed, 16 Jun 2021 15:00:37 +0100 Subject: [PATCH 4/4] Move client generation out of delegate --- tempodb/backend/azure/azure_helpers.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tempodb/backend/azure/azure_helpers.go b/tempodb/backend/azure/azure_helpers.go index b1daf9adf93..a2c6d48fe7d 100644 --- a/tempodb/backend/azure/azure_helpers.go +++ b/tempodb/backend/azure/azure_helpers.go @@ -32,20 +32,20 @@ func GetContainerURL(ctx context.Context, cfg *Config, hedge bool) (blob.Contain retryOptions.TryTimeout = time.Until(deadline) } - httpSender := pipeline.FactoryFunc(func(next pipeline.Policy, po *pipeline.PolicyOptions) pipeline.PolicyFunc { - return func(ctx context.Context, request pipeline.Request) (pipeline.Response, error) { + customTransport := http.DefaultTransport.(*http.Transport).Clone() - customTransport := http.DefaultTransport.(*http.Transport).Clone() + // add instrumentation + transport := newInstrumentedTransport(customTransport) - // add instrumentation - transport := newInstrumentedTransport(customTransport) + // hedge if desired (0 means disabled) + if hedge && cfg.HedgeRequestsAt != 0 { + transport = hedgedhttp.NewRoundTripper(cfg.HedgeRequestsAt, uptoHedgedRequests, transport) + } - // hedge if desired (0 means disabled) - if hedge && cfg.HedgeRequestsAt != 0 { - transport = hedgedhttp.NewRoundTripper(cfg.HedgeRequestsAt, uptoHedgedRequests, transport) - } + client := http.Client{Transport: transport} - client := http.Client{Transport: transport} + httpSender := pipeline.FactoryFunc(func(next pipeline.Policy, po *pipeline.PolicyOptions) pipeline.PolicyFunc { + return func(ctx context.Context, request pipeline.Request) (pipeline.Response, error) { // Send the request over the network resp, err := client.Do(request.WithContext(ctx))