prometheus / client_golang

Prometheus instrumentation library for Go applications
https://pkg.go.dev/github.com/prometheus/client_golang
Apache License 2.0
5.44k stars 1.19k forks source link

Sporadic error happening: label ... was collected before with the same name and label values #1269

Open ryap-pepperstone opened 1 year ago

ryap-pepperstone commented 1 year ago

Hi Team!

This error has been happening sporadically in a live environment. I am unable to replicate the issue using unit tests. My error could be related to this guy but i will be adding more details

I am using v1.15.0 with Golang 1.19.3. I am also using fiber golang v2.44.0 to expose the metrics endpoint. To convert the prometheus handler to a fiber handler, i am using gofiber adaptor v2.2.1

This is as far as i can trace it. I am unable to comprehend the things inside the Gather() function due to my limited knowledge of the library. That function is returning an error.

Raw Error

An error has occurred while serving metrics:

1 error(s) occurred:

* collected metric "NAMESPACE_SUBSYTEM_requests_total" { label:<name:"method" value:"PATCH" > label:<name:"path" value:"/api/someService/check" > label:<name:"servicename" value:"some-service" > label:<name:"status_code" value:"200" > counter:<value:1 > } was collected before with the same name and label values

Bits and pieces of my implementation How i am registering the metrics

func NewProm(..., reg *prometheus.Registry, ...) SomeInterface {

    // Ommitted code (declarations, vars, and etc...)

    reqTotal := promauto.With(reg).NewCounterVec(
        prometheus.CounterOpts{
            Name:        prometheus.BuildFQName(namespace, subsystem, "request_total"),
            Help:        "Count all http requests by status code, method and path.",
            ConstLabels: constantLabels,
        },
        defaultLabels,
    )

    // Register default client_golang metrics
    reg.MustRegister(
        collectors.NewGoCollector(),
    )

    return &prom{
        // More ommitted code ...
        reqTotal:        reqTotal, // For incrementing countervec later on
        reg:             reg,
    }
}

How i am registering the GET endpoint

func (s *prom) MakeGetMetricsHandler(app *fiber.App) {

    promHttpHandlerOpts = promhttp.HandlerOpts{
        Registry:          s.reg,
        ErrorLog:          NewPrometheusErrorLogger(s.logger), // Some logger. Nothing special
        EnableOpenMetrics: true,
    }

    promHandler := promhttp.HandlerFor(s.reg, promHttpHandlerOpts)
    app.Get(s.metricsURL, adaptor.HTTPHandler(promHandler))
}

Function to increment my metric

func (s *prom) IncReqTotal(statusCode string, method string, path string) {
    s.reqTotal.WithLabelValues(statusCode, method, path).Inc()
}

Middleware where the metric gets incremented

func (s *prom) Middleware() fiber.Handler {

    return func(ctx *fiber.Ctx) (err error) {
        err = ctx.Next()
        if err != nil {
            // Do some logging
        }

        // Get HTTP method type: GET|POST|PUT|etc
        method := ctx.Method()

        // Get raw route path: /api/path/etc
        path := ctx.Route().Path

        if path == s.metricsURL {
            // Skip if we're hitting the metrics url. We don't want to log hitting the metrics endpoint
            return err
        }

        // Get status code of request
        statusCode := strconv.Itoa(ctx.Response().StatusCode())

        // Increment req total
        s.IncReqTotal(statusCode, method, path)

        return err
    }

}

Any help would be appreciated! Thank you!

marevers commented 1 year ago

The error seems to be pointing towards an error in your logic somewhere. In your examples the snippet where you are incrementing your countervec is missing. I've seen this error in one of my implementations before too, and this occurred when trying to set a metric with a labelset (thus a vector) with duplicate label values in them. This results in a metric with the same name and label values, but a different value and thus that error is thrown.

ryap-pepperstone commented 1 year ago

@marevers I edited my post and added the bits of code mentioned: function that increments my metric, and the middleware that calls it

The labels i am adding to my metric are statusCode, method, and path. A sample request coming in would have the values 200, GET, and /api/somepath. I am not sure how any of the labels would get a duplicate value

liubq919 commented 1 year ago

same errors, try lots of ways, but errors still exist. switch to use gin framework, all errors gone, maybe fiber is the source of errors.

bwplotka commented 1 year ago

Hey, those kind of errors are typically happening with custom collectors with duplicated constant metrics by author. It's weird that here, it looks like it occurs with normal metrics. Normally if you duplicate new basic metric, the register will panic or error, depending if you use Must or not. To uncover what's happening a simple go file we can run locally, with reproduction of this bug would be amazing.

@liubq919 could help us creating one, since you say you can repro it with fiber?

YuriyChmil commented 1 year ago

Same problem with fiber. Has anyone successfully resolved this issue?

redstar01 commented 1 year ago

Hi, the same problem for me in the project with fiber framework and similar connection.

redstar01 commented 1 year ago

@ryap-pepperstone @YuriyChmil @liubq919 Hi! can you try using

method := ctx.Route().Method 

to collect method data instead of your variant

method := ctx.Method()
yalon commented 1 year ago

We too encountered something similar, and it looks like issue isn't on prometheus side - it's Fiber.

Basically Fiber returns strings that aren't really immutable. They added it to their documentation - a lot of discussion around "immutable": https://docs.gofiber.io (and this issue around it https://github.com/gofiber/fiber/issues/185)

I have to say though from Fiber's side it's still very error-prone because strings in Go are considered immutable so it's easy to make a mistake.

mihail641 commented 9 months ago

@ryap-pepperstone, you are using Fiber, in Fiber http parameters, body unsafe and mutable. Set Immutable: true in fiber.Config. Or you can copy unsafe string value in immutable string fmt.Sprinf("%s", myVal)