Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
ryap-pepperstone opened this issue May 5, 2023 · 9 comments
Labels

Comments

@ryap-pepperstone
Copy link

ryap-pepperstone commented May 5, 2023

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
Copy link
Contributor

marevers commented May 5, 2023

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
Copy link
Author

ryap-pepperstone commented May 5, 2023

@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
Copy link

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
Copy link
Member

bwplotka commented Jun 21, 2023

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
Copy link

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

@redstar01
Copy link

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

@redstar01
Copy link

redstar01 commented Aug 10, 2023

@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
Copy link

yalon commented Aug 28, 2023

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 gofiber/fiber#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
Copy link

@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)

@dosubot dosubot bot added the stale label Aug 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants