Skip to content

Commit

Permalink
move global metrics from gin router to runserver level
Browse files Browse the repository at this point in the history
update configs to use global layer instead of router layer
  • Loading branch information
dhontecillas committed Jan 29, 2024
1 parent 2fc4279 commit 58b2a24
Show file tree
Hide file tree
Showing 30 changed files with 440 additions and 489 deletions.
12 changes: 6 additions & 6 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -260,13 +260,13 @@ The following metrics are enabled if `round_trip` is set to true, and share the

###### Read Payload metrics

- `readed-size`: counted with the readed bytes (**TODO** should we remove this onw as is redundant with
- `read-size`: counted with the read bytes (**TODO** should we remove this onw as is redundant with
the histogram).
- `readed-size-hist`: histogram with the readed bytes
- `readed-time`: counter with seconds spent reading the body payload of the response.(**TODO** remove
- `read-size-hist`: histogram with the read bytes
- `read-time`: counter with seconds spent reading the body payload of the response.(**TODO** remove
if this is redundnat wiht the `reqded-time-hist`).
- `readed-time-hist`: histogram with the seconds spent reading the bdy.
- `readed-errors`: counter of number of errors that happened reading the response body.
- `read-time-hist`: histogram with the seconds spent reading the bdy.
- `read-errors`: counter of number of errors that happened reading the response body.

###### Detailed connection metrics

Expand Down Expand Up @@ -294,7 +294,7 @@ The following metrics are enabled if `detailed_connection` is set to **_true_**,
that has been canceled (usually when parallel requests are used).
- `error`: in case an error happened, the description of the error.

**readed-tracer** span with when `read_paylaod` option is set to true.
**read-tracer** span with when `read_paylaod` option is set to true.

### Instance

Expand Down
19 changes: 9 additions & 10 deletions config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ import (

// Config is the root configuration for the OTEL observability stack
type Config struct {
ServiceName string `json:"service_name"`
Layers *LayersOpts `json:"layers"`
Exporters []Exporter `json:"exporters"`
Instance *Instance `json:"instance"`
SkipPaths []string `json:"skip_paths"`
Extra map[string]interface{} `json:"extra"`
ServiceName string `json:"service_name"`
Layers *LayersOpts `json:"layers"`
Exporters []Exporter `json:"exporters"`
Instance *Instance `json:"instance"`
SkipPaths []string `json:"skip_paths"`
}

// Exporter has the inforamtion to configure an exporter
Expand Down Expand Up @@ -43,16 +42,16 @@ type Instance struct {
// LayersOpts contains the level of telemetry detail
// that we want for each KrakenD stage
type LayersOpts struct {
Router *RouterOpts `json:"router"`
Global *GlobalOpts `json:"global"`
Pipe *PipeOpts `json:"pipe"`
Backend *BackendOpts `json:"backend"`
}

// RouterOpts has the options for the KrakenD
// router stage.
// GlobalOpts has the options for the KrakenD
// http handler stage.
// We can select if we want to disable the metrics,
// the traces, and / or the trace propagation.
type RouterOpts struct {
type GlobalOpts struct {
DisableMetrics bool `json:"disable_metrics"`
DisableTraces bool `json:"disable_traces"`
DisablePropagation bool `json:"disable_propagation"`
Expand Down
4 changes: 2 additions & 2 deletions config/lura.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ func FromLura(srvCfg config.ServiceConfig) (*Config, error) {
cfg.Layers = &LayersOpts{}
}

if cfg.Layers.Router == nil {
cfg.Layers.Router = &RouterOpts{
if cfg.Layers.Global == nil {
cfg.Layers.Global = &GlobalOpts{
DisableMetrics: false,
DisableTraces: false,
DisablePropagation: false,
Expand Down
3 changes: 3 additions & 0 deletions doc/TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,6 @@
- allow different formats for the propagation of the trace (the `TextMapPropagator`)
in [endpoint.go](../router/gin/endpoint.go).
- review how we pass the state.

- we cannot use `skipPaths` with the same value that we have to define the endpoints
at the global layer, because we cannot know the matched pattern
2 changes: 1 addition & 1 deletion doc/implementation_details.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
How the code is organized:

- [io package](../io): has the wrappers for basic io classes to instrument the
bytes readed / written with an `io.Reader` / `io.Writer` (event the
bytes read / written with an `io.Reader` / `io.Writer` (event the
writer is not currently used in other packages)

- [http package](../http): has the common functions used to instrument an http
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
"telemetry/opentelemetry": {
"service_name": "krakend_back_service",
"layers": {
"router": {
"global": {
"disable_metrics": false,
"disable_traces": false,
"disable_propagation": false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
"telemetry/opentelemetry": {
"service_name": "krakend_frontend_service",
"layers": {
"router": {
"global": {
"disable_metrics": false,
"disable_traces": false,
"disable_propagation": false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"telemetry/opentelemetry": {
"service_name": "krakend_middle_service",
"layers": {
"router": {
"global": {
"disable_metrics": false,
"disable_traces": false,
"disable_propagation": false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
"telemetry/opentelemetry": {
"service_name": "krakend_backend_service",
"layers": {
"router": {
"global": {
"disable_metrics": false,
"disable_traces": false,
"disable_propagation": false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"telemetry/opentelemetry": {
"service_name": "krakend_frontend_service",
"layers": {
"router": {
"global": {
"disable_metrics": false,
"disable_traces": false,
"disable_propagation": false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"telemetry/opentelemetry": {
"service_name": "krakend_middle_service",
"layers": {
"router": {
"global": {
"disable_metrics": false,
"disable_traces": false,
"disable_propagation": false
Expand Down
14 changes: 7 additions & 7 deletions example/make_requests.sh
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
#!/bin/bash

# for i in {1..10}
for i in {1..1000}
for i in {1..1}
# for i in {1..1000}
do
curl localhost:54444/fake/fsf
curl localhost:54444/combination/2
# curl localhost:54444/fake/fsf
# curl localhost:54444/combination/2
curl localhost:54444/combination/1
curl localhost:54444/direct/slow
# curl localhost:54444/direct/slow
sleep 0.1
curl localhost:54444/direct/delayed
curl localhost:54444/direct/drop
# curl localhost:54444/direct/delayed
# curl localhost:54444/direct/drop
done

# curl localhost:44444/fake/fsf | jq
Expand Down
25 changes: 17 additions & 8 deletions example/server/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ import (
"github.com/luraproject/lura/v2/transport/http/server"

kotel "github.com/krakend/krakend-otel"
// "github.com/krakend/krakend-otel/exporter"
kotelconfig "github.com/krakend/krakend-otel/config"
"github.com/krakend/krakend-otel/lura"
otellura "github.com/krakend/krakend-otel/lura"
otelgin "github.com/krakend/krakend-otel/router/gin"
"github.com/krakend/krakend-otel/state"
)
Expand Down Expand Up @@ -91,25 +90,35 @@ func main() {
otelStateFn := state.GlobalState

bf := func(backendConfig *config.Backend) proxy.Proxy {
reqExec := lura.HTTPRequestExecutorFromConfig(client.NewHTTPClient,
reqExec := otellura.HTTPRequestExecutorFromConfig(client.NewHTTPClient,
backendConfig, obsConfig.Layers.Backend, obsConfig.SkipPaths, otelStateFn)
return proxy.NewHTTPProxyWithHTTPExecutor(backendConfig, reqExec, backendConfig.Decoder)
}
bf = lura.BackendFactory(bf, otelStateFn, obsConfig.Layers.Backend, obsConfig.SkipPaths)
bf = otellura.BackendFactory(bf, otelStateFn, obsConfig.Layers.Backend, obsConfig.SkipPaths)

defaultPF := proxy.NewDefaultFactory(bf, logger)
pf := lura.ProxyFactory(defaultPF, otelStateFn, obsConfig.Layers.Pipe, obsConfig.SkipPaths)
pf := otellura.ProxyFactory(defaultPF, otelStateFn, obsConfig.Layers.Pipe, obsConfig.SkipPaths)

handlerF := otelgin.New(krakendgin.EndpointHandler, otelStateFn,
obsConfig.Layers.Router, obsConfig.SkipPaths)
obsConfig.Layers.Global, obsConfig.SkipPaths)

runserverChain := krakendgin.RunServerFunc(
otellura.GlobalRunServer(logger, obsConfig, otelStateFn, server.RunServer))

engine := gin.Default()
engine.RedirectTrailingSlash = true
engine.RedirectFixedPath = true
engine.HandleMethodNotAllowed = true
engine.ContextWithFallback = true // <- this is important for trace span propagation

// setup the krakend router
routerFactory := krakendgin.NewFactory(krakendgin.Config{
Engine: gin.Default(),
Engine: engine,
ProxyFactory: pf,
Middlewares: []gin.HandlerFunc{},
Logger: logger,
HandlerFactory: handlerF,
RunServer: server.RunServer,
RunServer: runserverChain,
})

// start the engine
Expand Down
22 changes: 11 additions & 11 deletions http/client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestInstrumentedHTTPClient(t *testing.T) {

b, _ := io.ReadAll(resp.Body)
if len(b) == 0 {
t.Errorf("no bytes readed")
t.Errorf("no bytes read")
return
}

Expand Down Expand Up @@ -163,10 +163,10 @@ func TestInstrumentedHTTPClient(t *testing.T) {
"requests-content-length": false,
"response-latency": false,
"response-content-length": false,
"readed-size": false,
"readed-size-hist": false,
"readed-time": false,
"readed-time-hist": false,
"read-size": false,
"read-size-hist": false,
"read-time": false,
"read-time-hist": false,
// "reader-errors": false,
}
numWantedMetrics := len(wantedMetrics)
Expand All @@ -189,17 +189,17 @@ func TestInstrumentedHTTPClient(t *testing.T) {
}

// --> check that the metrics have the expected attributes set
readedSize := gotMetrics["readed-size"]
readedSizeSum, ok := readedSize.Data.(metricdata.Sum[int64])
readSize := gotMetrics["read-size"]
readSizeSum, ok := readSize.Data.(metricdata.Sum[int64])
if !ok {
t.Errorf("cannot access readed size aggregation: %#v", readedSize.Data)
t.Errorf("cannot access read size aggregation: %#v", readSize.Data)
return
}
if len(readedSizeSum.DataPoints) != 1 {
t.Errorf("readed sum data points, want: 1, got: %d", len(readedSizeSum.DataPoints))
if len(readSizeSum.DataPoints) != 1 {
t.Errorf("read sum data points, want: 1, got: %d", len(readSizeSum.DataPoints))
return
}
dp := readedSizeSum.DataPoints[0]
dp := readSizeSum.DataPoints[0]

if dp.Attributes.Len() != 1 {
t.Errorf("missing attributes, want 1, got: %d\n%#v", dp.Attributes.Len(), dp.Attributes)
Expand Down
28 changes: 14 additions & 14 deletions router/gin/endpoint_metrics.go → http/server/metrics.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
package gin
package server

import (
"go.opentelemetry.io/otel/attribute"
Expand Down Expand Up @@ -28,33 +28,33 @@ var (
)
)

type ginMetrics struct {
type metricsHTTP struct {
fixedAttrs []attribute.KeyValue
fixedAttrsOpts metric.MeasurementOption

latency metric.Float64Histogram
size metric.Int64Histogram
}

func newGinMetrics(meter metric.Meter, attrs []attribute.KeyValue) *ginMetrics {
var gm ginMetrics
gm.latency, _ = meter.Float64Histogram("router-response-latency", timeBucketsOpt)
gm.size, _ = meter.Int64Histogram("router-response-size", sizeBucketsOpt)
func newMetricsHTTP(meter metric.Meter, attrs []attribute.KeyValue) *metricsHTTP {
var m metricsHTTP
m.latency, _ = meter.Float64Histogram("global-response-latency", timeBucketsOpt)
m.size, _ = meter.Int64Histogram("global-response-size", sizeBucketsOpt)
if len(attrs) > 0 {
gm.fixedAttrs = make([]attribute.KeyValue, len(attrs))
copy(gm.fixedAttrs, attrs)
gm.fixedAttrsOpts = metric.WithAttributeSet(attribute.NewSet(gm.fixedAttrs...))
m.fixedAttrs = make([]attribute.KeyValue, len(attrs))
copy(m.fixedAttrs, attrs)
m.fixedAttrsOpts = metric.WithAttributeSet(attribute.NewSet(m.fixedAttrs...))
}
return &gm
return &m
}

func (m *ginMetrics) report(ht *handlerTracking) {
func (m *metricsHTTP) report(t *tracking) {
if m == nil || m.latency == nil {
return
}
dynAttrsOpts := metric.WithAttributes(
semconv.HTTPResponseStatusCode(ht.responseStatus),
semconv.HTTPResponseStatusCode(t.responseStatus),
)
m.latency.Record(ht.ctx, ht.latencyInSecs, m.fixedAttrsOpts, dynAttrsOpts)
m.size.Record(ht.ctx, int64(ht.responseSize), m.fixedAttrsOpts, dynAttrsOpts)
m.latency.Record(t.ctx, t.latencyInSecs, m.fixedAttrsOpts, dynAttrsOpts)
m.size.Record(t.ctx, int64(t.responseSize), m.fixedAttrsOpts, dynAttrsOpts)
}
55 changes: 55 additions & 0 deletions http/server/response_writer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package server

import (
"bufio"
"fmt"
"net"
"net/http"
)

type TrackingResponseWriter struct {
track *tracking
rw http.ResponseWriter
flusher http.Flusher
hijacker http.Hijacker
}

func (w *TrackingResponseWriter) Header() http.Header {
return w.rw.Header()
}

func (w *TrackingResponseWriter) Write(b []byte) (int, error) {
nBytes, e := w.rw.Write(b)
if e != nil {
w.track.writeErrs = append(w.track.writeErrs, e)
}
w.track.responseSize += nBytes
return nBytes, e
}

func (w *TrackingResponseWriter) WriteHeader(statusCode int) {
w.track.responseStatus = statusCode
w.rw.WriteHeader(statusCode)
}

func (w *TrackingResponseWriter) Hijack() (net.Conn, *bufio.ReadWriter, error) {
if w.hijacker != nil {
return w.hijacker.Hijack()
}
return nil, nil, fmt.Errorf("not implemented")
}

func (w *TrackingResponseWriter) Flush() {
if w.flusher != nil {
w.flusher.Flush()
}
}

func newTrackingResponseWriter(rw http.ResponseWriter, t *tracking) *TrackingResponseWriter {
return &TrackingResponseWriter{
track: t,
rw: rw,
flusher: rw.(http.Flusher),
hijacker: rw.(http.Hijacker),
}
}
Loading

0 comments on commit 58b2a24

Please sign in to comment.