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

It occurs "collected metric %s %s was collected before with the same name and label values" error in the lastest version #242

Closed
nature502 opened this issue Oct 15, 2016 · 11 comments
Assignees
Labels

Comments

@nature502
Copy link

We use prometheus to monitor our golang application. It works well. When we upgrade to release 0.8.0 (commit ID c5b7fcc). It occurs "collected metric %s %s was collected before with the same name and label values" error. I find the old version ignores consistency checks, but the lastest must do it . So, when we use counter to count, it occurs errors.
Code in previous:
// EnableCollectChecks enables (or disables) additional consistency checks
// during metrics collection. These additional checks are not enabled by default
// because they inflict a performance penalty and the errors they check for can
// only happen if the used Metric and Collector types have internal programming
// errors. It can be helpful to enable these checks while working with custom
// Collectors or Metrics whose correctness is not well established yet.
func EnableCollectChecks(b bool) {
defRegistry.collectChecksEnabled = b
}

// decide skip or not.
if r.collectChecksEnabled {
if err := r.checkConsistency(metricFamily, dtoMetric, desc, metricHashes); err != nil {
return 0, err
}
}

Code in lastest
// Gather.
for metric := range metricChan {
.................
metricFamily, ok := metricFamiliesByName[desc.fqName]
if ok {
.............................
} else {
.......................................
if err := checkMetricConsistency(metricFamily, dtoMetric, metricHashes, dimHashes); err != nil {
errs = append(errs, err
continue
}

I think problem is here. It ignores locking of metricFamiliesByName in concurrent environment.

@beorn7 beorn7 self-assigned this Oct 17, 2016
@beorn7 beorn7 added the bug label Oct 17, 2016
@beorn7
Copy link
Member

beorn7 commented Oct 17, 2016

I don't see the problem. metricFamiliesByName is never used in a concurrent context. It's a local variable, and only used in the for loop where only one goroutine is acting on it.

Are you perhaps really collecting inconsistent metrics? Could you provide more detail on which metric the error message is triggered?

You also might have run into a hash collision, see #222 . That would be super interesting to hunt down.

@nature502
Copy link
Author

nature502 commented Oct 18, 2016

We use counter to count url hits. We have two urls. One is "/api/v1/app", the other is "/api/v1/ad". Below is the code in our application.

    webUrlHitCounts = prom.NewCounterVec(
        prom.CounterOpts{
            Namespace: "xxx",
            Subsystem: "web",
            Name:      "url_hit_counts",
            Help:      "xxx web url hit counts.",
        },
        []string{labelUrl}, // labelUrl = "url_path"
    )
       webUrlHitCounts.WithLabelValues(ctx.PathString()).Inc()

It occurs the below error in page.
url http://xxx.xxx.xxx.xxx:xxxx/_/metrics
**An error has occurred during metrics collection:

9 error(s) occurred:

  • collected metric xxx_web_url_hit_counts label:<name:"url_path" value:"/api/v1/adp" > counter:<value:6 > was collected before with the same name and label values
  • collected metric xxx_web_url_hit_counts label:<name:"url_path" value:"/api/v1/adp" > counter:<value:3 > was collected before with the same name and label values
  • collected metric xxx_web_url_hit_counts label:<name:"url_path" value:"/api/v1/ack" > counter:<value:2 > was collected before with the same name and label values
  • collected metric xxx_web_url_hit_counts label:<name:"url_path" value:"/api/v1/ack" > counter:<value:5 > was collected before with the same name and label values
  • collected metric xxx_web_url_hit_counts label:<name:"url_path" value:"/api/v1/ack" > counter:<value:3 > was collected before with the same name and label values
  • collected metric xxx_web_url_hit_counts label:<name:"url_path" value:"/api/v1/ac" > counter:<value:49 > was collected before with the same name and label values
  • collected metric xxx_web_url_hit_counts label:<name:"url_path" value:"/api/v1/ad" > counter:<value:6 > was collected before with the same name and label values
  • collected metric xxx_web_url_hit_counts label:<name:"url_path" value:"/api/v1/ac" > counter:<value:9 > was collected before with the same name and label values
  • collected metric xxx_web_url_hit_counts label:<name:"url_path" value:"/api/v1/app" > counter:<value:10091 > was collected before with the same name and label values.**

In addition, I find a very puzzled thing. We only count two urls, one is "/api/v1/app", the other is "api/v1/ad". The result shows others urls which we do not have. It occurs in previous and lastest version.

@beorn7
Copy link
Member

beorn7 commented Oct 18, 2016

In addition, I find a very puzzled thing. We only count two urls, one is "/api/v1/app", the other is "api/v1/ad". The result shows others urls which we do not have. It occurs in previous and lastest version.

This is a hint that you have some rogue collector in your code that feeds into the same metric. Only with v0.8.0, you would notice that. If you are really only using a stock CounterVec, the consistency check should never result in an error (unless there is a bug in the library or you have really run into a hash collision).

@beorn7
Copy link
Member

beorn7 commented Nov 3, 2016

I assume this is clarified. If not, please re-open with more evidence.

@beorn7 beorn7 closed this as completed Nov 3, 2016
@detailyang
Copy link

@beorn7 Same issue here, it's hard to debug it:(. I cannot construct the issue in the manual. Do you have any idea?

Many thanks

@beorn7
Copy link
Member

beorn7 commented May 31, 2019

@detailyang This issue was filed for a change introduced in v0.8 more than two years ago. It is unlikely that you have run into the same issue now.

You might have discovered an entirely new bug. In that case, you should file a new issue, providing the necessary context so that others can find the bug. (Ideally, you'll find it yourself.)

However, it seems you don't know yet why the error message shows up and you have questions how to understand why it is showing up. It makes more sense to ask questions like this on the prometheus-users mailing list rather than in a GitHub issue. (A closed GitHub issue is even less suited.) On the mailing list, more people are available to potentially respond to your question, and the whole community can benefit from the answers provided.

@detailyang
Copy link

@beorn7

I got an idea to get more information about this. It's hard to reproduce the strange issue but I will look at more. I will join the malling list to solve this with the whole community.

Anyway, many thanks:)

@ainiml
Copy link

ainiml commented Jul 31, 2020

Getting the same error

* collected metric "atIfIndex" { label:<name:"atIfIndex" value:"95" > label:<name:"atNetAddress" value:"1.10.10.10" > gauge:<value:95 > } was collected before with the same name and label values

@alexellis
Copy link

I ran into this because of:

	for _, totals := range queuedJobs {
		c.queuedJobsGauge.WithLabelValues(totals.Owner).Set(float64(totals.QueuedTotal))
	        c.queuedJobsGauge.Collect(ch)
	}

And it should have been:

	for _, totals := range queuedJobs {
		c.queuedJobsGauge.WithLabelValues(totals.Owner).Set(float64(totals.QueuedTotal))
	}

	c.queuedJobsGauge.Collect(ch)

Easy mistake to make and very hard to figure out.

christarazi pushed a commit to carnerito/cilium that referenced this issue Aug 28, 2023
Previously, the 'metricsmap-bpf-prom-sync' controller was responsible for periodically collecting cilium_datapath drop
and forward metrics.
This commit introduces the metricsmapCollector, which implements the Prometheus Collector interface within the
metricsmap package. As a result, the aforementioned controller has been removed.
The metricsmapCollector is registered within the global Prometheus registry during the initialization of metricsmap.
The metrics.Register function has been modified to propagate errors from the registry.Register function instead of
simply overriding it with nil.
The metricsmapCollector comprises two metrics maps: forwardedMetricsMap and droppedMetricsMap.
These maps are populated within a callback function passed to the IterateWithCallback function.
This approach serves two primary purposes:
1. Separation of Map Iteration and Metric Update: By separating the iteration over the BPF map and the updating of
   Prometheus metrics, the implementation ensures that no partial metrics are exposed in case of map iteration failure.
2. Normalization of exposed Metrics: Unlike the statement in the bpf/lib/metrics.h comments, which suggests exposing
   only one reason label for forwarded metrics, the eBPF code exposes multiple reasons. Through testing, it was found
   that reasons 0 and 3 were exposed which triggered this error in prometheus client:
   prometheus/client_golang#242

Fixes: cilium#27058

Signed-off-by: Boris Petrovic <carnerito.b@gmail.com>
christarazi pushed a commit to carnerito/cilium that referenced this issue Oct 3, 2023
Previously, the 'metricsmap-bpf-prom-sync' controller was responsible for periodically collecting cilium_datapath drop
and forward metrics.
This commit introduces the metricsmapCollector, which implements the Prometheus Collector interface within the
metricsmap package. As a result, the aforementioned controller has been removed.
The metricsmapCollector is registered within the global Prometheus registry during the initialization of metricsmap.
The metrics.Register function has been modified to propagate errors from the registry.Register function instead of
simply overriding it with nil.
The metricsmapCollector comprises two metrics maps: forwardedMetricsMap and droppedMetricsMap.
These maps are populated within a callback function passed to the IterateWithCallback function.
This approach serves two primary purposes:
1. Separation of Map Iteration and Metric Update: By separating the iteration over the BPF map and the updating of
   Prometheus metrics, the implementation ensures that no partial metrics are exposed in case of map iteration failure.
2. Normalization of exposed Metrics: Unlike the statement in the bpf/lib/metrics.h comments, which suggests exposing
   only one reason label for forwarded metrics, the eBPF code exposes multiple reasons. Through testing, it was found
   that reasons 0 and 3 were exposed which triggered this error in prometheus client:
   prometheus/client_golang#242

Furthermore, cilium command has indirect dependency on metricsmap package. To prevent metrics map initialization
to happen each time cilium command is executed, metricsmap is converted to hive Cell and injected in cilium daemon
Infrastructure module. For details see this comment: cilium#27370 (comment).

Fixes: cilium#27058

Signed-off-by: Boris Petrovic <carnerito.b@gmail.com>
carnerito added a commit to carnerito/cilium that referenced this issue Oct 17, 2023
Previously, the 'metricsmap-bpf-prom-sync' controller was responsible for periodically collecting cilium_datapath drop
and forward metrics.
This commit introduces the metricsmapCollector, which implements the Prometheus Collector interface within the
metricsmap package. As a result, the aforementioned controller has been removed.
The metricsmapCollector is registered within the global Prometheus registry during the initialization of metricsmap.
The metrics.Register function has been modified to propagate errors from the registry.Register function instead of
simply overriding it with nil.
The metricsmapCollector comprises two metrics maps: forwardedMetricsMap and droppedMetricsMap.
These maps are populated within a callback function passed to the IterateWithCallback function.
This approach serves two primary purposes:
1. Separation of Map Iteration and Metric Update: By separating the iteration over the BPF map and the updating of
   Prometheus metrics, the implementation ensures that no partial metrics are exposed in case of map iteration failure.
2. Normalization of exposed Metrics: Unlike the statement in the bpf/lib/metrics.h comments, which suggests exposing
   only one reason label for forwarded metrics, the eBPF code exposes multiple reasons. Through testing, it was found
   that reasons 0 and 3 were exposed which triggered this error in prometheus client:
   prometheus/client_golang#242

Furthermore, cilium command has indirect dependency on metricsmap package. To prevent metrics map initialization
to happen each time cilium command is executed, metricsmap is converted to hive Cell and injected in cilium daemon
Infrastructure module. For details see this comment: cilium#27370 (comment).

Fixes: cilium#27058

Signed-off-by: Boris Petrovic <carnerito.b@gmail.com>
christarazi pushed a commit to carnerito/cilium that referenced this issue Oct 30, 2023
Previously, the 'metricsmap-bpf-prom-sync' controller was responsible for periodically collecting cilium_datapath drop
and forward metrics.
This commit introduces the metricsmapCollector, which implements the Prometheus Collector interface within the
metricsmap package. As a result, the aforementioned controller has been removed.
The metricsmapCollector is registered within the global Prometheus registry during the initialization of metricsmap.
The metrics.Register function has been modified to propagate errors from the registry.Register function instead of
simply overriding it with nil.
The metricsmapCollector comprises two metrics maps: forwardedMetricsMap and droppedMetricsMap.
These maps are populated within a callback function passed to the IterateWithCallback function.
This approach serves two primary purposes:
1. Separation of Map Iteration and Metric Update: By separating the iteration over the BPF map and the updating of
   Prometheus metrics, the implementation ensures that no partial metrics are exposed in case of map iteration failure.
2. Normalization of exposed Metrics: Unlike the statement in the bpf/lib/metrics.h comments, which suggests exposing
   only one reason label for forwarded metrics, the eBPF code exposes multiple reasons. Through testing, it was found
   that reasons 0 and 3 were exposed which triggered this error in prometheus client:
   prometheus/client_golang#242

Furthermore, cilium command has indirect dependency on metricsmap package. To prevent metrics map initialization
to happen each time cilium command is executed, metricsmap is converted to hive Cell and injected in cilium daemon
Infrastructure module. For details see this comment: cilium#27370 (comment).

Fixes: cilium#27058

Signed-off-by: Boris Petrovic <carnerito.b@gmail.com>
aanm pushed a commit to carnerito/cilium that referenced this issue Oct 31, 2023
Previously, the 'metricsmap-bpf-prom-sync' controller was responsible for periodically collecting cilium_datapath drop
and forward metrics.
This commit introduces the metricsmapCollector, which implements the Prometheus Collector interface within the
metricsmap package. As a result, the aforementioned controller has been removed.
The metricsmapCollector is registered within the global Prometheus registry during the initialization of metricsmap.
The metrics.Register function has been modified to propagate errors from the registry.Register function instead of
simply overriding it with nil.
The metricsmapCollector comprises two metrics maps: forwardedMetricsMap and droppedMetricsMap.
These maps are populated within a callback function passed to the IterateWithCallback function.
This approach serves two primary purposes:
1. Separation of Map Iteration and Metric Update: By separating the iteration over the BPF map and the updating of
   Prometheus metrics, the implementation ensures that no partial metrics are exposed in case of map iteration failure.
2. Normalization of exposed Metrics: Unlike the statement in the bpf/lib/metrics.h comments, which suggests exposing
   only one reason label for forwarded metrics, the eBPF code exposes multiple reasons. Through testing, it was found
   that reasons 0 and 3 were exposed which triggered this error in prometheus client:
   prometheus/client_golang#242

Furthermore, cilium command has indirect dependency on metricsmap package. To prevent metrics map initialization
to happen each time cilium command is executed, metricsmap is converted to hive Cell and injected in cilium daemon
Infrastructure module. For details see this comment: cilium#27370 (comment).

Fixes: cilium#27058

Signed-off-by: Boris Petrovic <carnerito.b@gmail.com>
carnerito added a commit to carnerito/cilium that referenced this issue Oct 31, 2023
Previously, the 'metricsmap-bpf-prom-sync' controller was responsible for periodically collecting cilium_datapath drop
and forward metrics.
This commit introduces the metricsmapCollector, which implements the Prometheus Collector interface within the
metricsmap package. As a result, the aforementioned controller has been removed.
The metricsmapCollector is registered within the global Prometheus registry during the initialization of metricsmap.
The metrics.Register function has been modified to propagate errors from the registry.Register function instead of
simply overriding it with nil.
The metricsmapCollector comprises two metrics maps: forwardedMetricsMap and droppedMetricsMap.
These maps are populated within a callback function passed to the IterateWithCallback function.
This approach serves two primary purposes:
1. Separation of Map Iteration and Metric Update: By separating the iteration over the BPF map and the updating of
   Prometheus metrics, the implementation ensures that no partial metrics are exposed in case of map iteration failure.
2. Normalization of exposed Metrics: Unlike the statement in the bpf/lib/metrics.h comments, which suggests exposing
   only one reason label for forwarded metrics, the eBPF code exposes multiple reasons. Through testing, it was found
   that reasons 0 and 3 were exposed which triggered this error in prometheus client:
   prometheus/client_golang#242

Furthermore, cilium command has indirect dependency on metricsmap package. To prevent metrics map initialization
to happen each time cilium command is executed, metricsmap is converted to hive Cell and injected in cilium daemon
Infrastructure module. For details see this comment: cilium#27370 (comment).

Fixes: cilium#27058

Signed-off-by: Boris Petrovic <carnerito.b@gmail.com>
aanm pushed a commit to cilium/cilium that referenced this issue Nov 2, 2023
Previously, the 'metricsmap-bpf-prom-sync' controller was responsible for periodically collecting cilium_datapath drop
and forward metrics.
This commit introduces the metricsmapCollector, which implements the Prometheus Collector interface within the
metricsmap package. As a result, the aforementioned controller has been removed.
The metricsmapCollector is registered within the global Prometheus registry during the initialization of metricsmap.
The metrics.Register function has been modified to propagate errors from the registry.Register function instead of
simply overriding it with nil.
The metricsmapCollector comprises two metrics maps: forwardedMetricsMap and droppedMetricsMap.
These maps are populated within a callback function passed to the IterateWithCallback function.
This approach serves two primary purposes:
1. Separation of Map Iteration and Metric Update: By separating the iteration over the BPF map and the updating of
   Prometheus metrics, the implementation ensures that no partial metrics are exposed in case of map iteration failure.
2. Normalization of exposed Metrics: Unlike the statement in the bpf/lib/metrics.h comments, which suggests exposing
   only one reason label for forwarded metrics, the eBPF code exposes multiple reasons. Through testing, it was found
   that reasons 0 and 3 were exposed which triggered this error in prometheus client:
   prometheus/client_golang#242

Furthermore, cilium command has indirect dependency on metricsmap package. To prevent metrics map initialization
to happen each time cilium command is executed, metricsmap is converted to hive Cell and injected in cilium daemon
Infrastructure module. For details see this comment: #27370 (comment).

Fixes: #27058

Signed-off-by: Boris Petrovic <carnerito.b@gmail.com>
@Nachtfalkeaw
Copy link

Hello,
I maybe have the same error with some pfsense MIBs and atest snmp_exporter 0.25.0 and its generator:

An error has occurred while serving metrics:

3 error(s) occurred:

  • collected metric "begemotIfMaxspeed" { counter:{value:0}} was collected before with the same name and label values
  • collected metric "begemotIfMaxspeed" { counter:{value:1e+09}} was collected before with the same name and label values
  • collected metric "begemotIfMaxspeed" { counter:{value:100}} was collected before with the same name and label values

I try to walk these:
walk: [sysUpTime,pfInterfaces,pfTablesTblNumber,pfTablesTblTable,pfCounter,pfStateTable,pfStatus,freeBSDsrc,begemotIp,ipv4InterfaceTableLastChange,ipv4InterfaceTable,ipv4InterfaceEntry,ipv4InterfaceIfIndex,ipTrafficStats,ipIfStatsTable,ipMIBConformance]

@mihail641
Copy link

@nature502 , @detailyang , @ainiml , @Nachtfalkeaw , If you 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) for label values.

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

7 participants