feat: set OTEL global providers

[adigiorgi-clickup]

Fixes #21

[dhontecillas]

Well, I am not sure this is a good a idea to put it into a library: if any library would set the otel global variable, you would end up with a mess, I think. I would rather put those two lines of code in the krakend-ce itself. The effect would be the same, but krakend-ce is a binary and makes sense to set that value at the executable level. What do you think ?

[adigiorgi-clickup]

But IMHO only the part of KrakenD that creates meter/tracer providers should be the one setting them as global after creating them. If we move it into krakend-ce, doesn't it mean that krakend-ce becomes dependent on OTEL? But I never pushed code to krakend-ce, I don't know how much you want to keep things separate between the library and the app 😄

[adigiorgi-clickup]

@dhontecillas, sorry for the ping. How should we proceed with this PR? Just wanted to keep the conversation going 😄

[TerraSkye]

shouldn't we add the provider in the context? so that we can retrieve it from there?

[adigiorgi-clickup]

@TerraSkye, maybe, but it seems that having global providers is the way OTEL prefers it 🤷‍♂️

[kpacha]

Unfortunately, this PR won't fix the #21 because the fqn of this package as a dependency of the binary and as a dependency of the plugin won't be the same, so the package variables won't be pointing to the same memory space

[adigiorgi-clickup]

@kpacha, but when I created a plugin, imported github.com/krakend/krakend-otel/state, and called state.GlobalState(), I got the actual provider. So it seems that the memory space is shared between plugins and the main KrakenD binary 🤔

[adigiorgi-clickup]

My plugin is more or less something like this:

package main

import (
    "crypto/tls"
    "encoding/json"
    "errors"
    "reflect"

    "github.com/go-redis/redis_rate/v10"
    "github.com/krakend/krakend-otel/state"
    "github.com/redis/go-redis/extra/redisotel/v9"
    "github.com/redis/go-redis/v9"
)

type modifierRegisterer string

var (
    ModifierRegisterer        = modifierRegisterer(pluginName)
)

func (r modifierRegisterer) RegisterModifiers(f func(
    name string,
    factoryFunc func(map[string]interface{}) func(interface{}) (interface{}, error),
    appliesToRequest bool,
    appliesToResponse bool,
)) {
    f(string(r), r.getModifyRequest, true, false)
}

func (r modifierRegisterer) getModifyRequest(
    cfg map[string]interface{},
) func(interface{}) (interface{}, error) {
    pluginCfg, ok := cfg[string(r)].(map[string]interface{})
    if !ok {
        logger.Fatal("Configuration not found")
    }

    m, err := r.getRateLimitRequestModifier(pluginCfg)
    if err != nil {
        logger.Fatal(err)
    }

    return m.ModifyRequest
}

func (r modifierRegisterer) getRateLimitRequestModifier(pluginCfg map[string]interface{}) (*RateLimitRequestModifier, error) {
    // EXTRA STUFF  

    rdb := redis.NewClient(rdbOptions)

    otelState := state.GlobalState()

    // See https://stackoverflow.com/questions/13476349/check-for-nil-and-nil-interface-in-go
    if otelState != nil && !reflect.ValueOf(otelState).IsNil() {
        meterProvider := otelState.MeterProvider()
        if err := redisotel.InstrumentMetrics(rdb, redisotel.WithMeterProvider(meterProvider)); err != nil {
            return nil, err
        }

        tracerProvider := otelState.TracerProvider()
        if err := redisotel.InstrumentTracing(rdb, redisotel.WithTracerProvider(tracerProvider)); err != nil {
            return nil, err
        }
    }

    limiter := redis_rate.NewLimiter(rdb)

    m = &RateLimitRequestModifier{
        limit:         redis_rate.PerMinute(int(limitPerMinute)),
        limiter:       limiter,
        skippedUserId: skippedUserId,
    }

    rateLimitRequestModifiers[cfgKey] = m

    logger.Debug("Rate limit request modifier created")

    return m, nil
}

[kpacha]

@adigiorgi-clickup that's weird, because that restriction was in place since the plugin package of the go stdlib was published. This conditioned the plugin loading and injecting for krakend and it is why we deal with local interfaces instead of sharing things like logging.Logger, proxy.Request and proxy.Response directly.

The most weird part for me is: that package was freeze several minors ago (aka several years) so there should be no code changes, so I'll need to check if anything else changed.

I'll do a quick test to verify your statement. If the PR works, I'll be more than happy to accept it. Can you also share the go.mod of your plugin? That will help me with the recreation of your environment

[adigiorgi-clickup]

@kpacha, sure, here it is!

module github.com/time-loop/clickup/libs/gateway/plugin-rate-limit

go 1.22.2

require (
    github.com/go-redis/redis_rate/v10 v10.0.1
    github.com/krakend/krakend-otel v0.5.0
    github.com/redis/go-redis/extra/redisotel/v9 v9.0.5
    github.com/redis/go-redis/v9 v9.0.5
    github.com/stretchr/testify v1.9.0
)

require (
    github.com/beorn7/perks v1.0.1 // indirect
    github.com/cenkalti/backoff/v4 v4.2.1 // indirect
    github.com/cespare/xxhash/v2 v2.2.0 // indirect
    github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect
    github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect
    github.com/go-logr/logr v1.4.1 // indirect
    github.com/go-logr/stdr v1.2.2 // indirect
    github.com/grpc-ecosystem/grpc-gateway/v2 v2.19.1 // indirect
    github.com/luraproject/lura/v2 v2.6.3 // indirect
    github.com/pmezard/go-difflib v1.0.0 // indirect
    github.com/prometheus/client_golang v1.19.0 // indirect
    github.com/prometheus/client_model v0.6.1 // indirect
    github.com/prometheus/common v0.52.2 // indirect
    github.com/prometheus/procfs v0.13.0 // indirect
    github.com/redis/go-redis/extra/rediscmd/v9 v9.0.5 // indirect
    github.com/stretchr/objx v0.5.2 // indirect
    go.opentelemetry.io/otel v1.25.0 // indirect
    go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetricgrpc v1.24.0 // indirect
    go.opentelemetry.io/otel/exporters/otlp/otlpmetric/otlpmetrichttp v1.24.0 // indirect
    go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.24.0 // indirect
    go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.24.0 // indirect
    go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.24.0 // indirect
    go.opentelemetry.io/otel/exporters/prometheus v0.47.0 // indirect
    go.opentelemetry.io/otel/metric v1.25.0 // indirect
    go.opentelemetry.io/otel/sdk v1.25.0 // indirect
    go.opentelemetry.io/otel/sdk/metric v1.25.0 // indirect
    go.opentelemetry.io/otel/trace v1.25.0 // indirect
    go.opentelemetry.io/proto/otlp v1.2.0 // indirect
    golang.org/x/net v0.24.0 // indirect
    golang.org/x/sys v0.19.0 // indirect
    golang.org/x/text v0.14.0 // indirect
    google.golang.org/genproto/googleapis/api v0.0.0-20240401170217-c3f982113cda // indirect
    google.golang.org/genproto/googleapis/rpc v0.0.0-20240401170217-c3f982113cda // indirect
    google.golang.org/grpc v1.63.2 // indirect
    google.golang.org/protobuf v1.33.0 // indirect
    gopkg.in/yaml.v3 v3.0.1 // indirect
)

[aaron-weisberg-qz]

Any update on this?