bug: LabelGuardedCounter metrics report incorrect value

[fuyufjh]

Describe the bug

Related #13080

Error message/log

No response

To Reproduce

No response

Expected behavior

No response

How did you deploy RisingWave?

No response

The version of RisingWave

main

Additional context

No response

[hzxa21]

cc @wenym1

[wenym1]

Is there any query to reproduce the bug?

[fuyufjh]

Is there any query to reproduce the bug?

Try this:

CREATE TABLE t1 (k int, v1 struct<v2 int, v3 double>, t1 timestamp, c1 varchar) 
WITH (
     connector = 'datagen',
  
     fields.k.kind = 'sequence',
     fields.k.start = '1',
  
     fields.v1.v2.kind = 'random',
     fields.v1.v2.min = '-10',
     fields.v1.v2.max = '10',
     fields.v1.v2.seed = '1',
  
     fields.v1.v3.kind = 'random',
     fields.v1.v3.min = '15',
     fields.v1.v3.max = '55',
     fields.v1.v3.seed = '1',
  
     fields.t1.kind = 'random',
     fields.t1.max_past = '2h 37min',
     fields.t1.max_past_mode = 'relative',
     fields.t1.seed = '3',

     fields.z1.kind = 'random',
     fields.z1.max_past = '2h 37min',
     fields.z1.max_past_mode = 'relative',
     fields.z1.seed = '3',

     fields.c1.kind = 'random',
     fields.c1.length = '16',
     fields.c1.seed = '3',
  
     datagen.rows.per.second = '10'
 ) FORMAT PLAIN ENCODE JSON;


create materialized view mv1 as select (v1).v2, count(k) from t1 group by 1;

And then observe the actor_out_record_cnt from Grafana.

[fuyufjh]

I am suspecting

risingwave/src/common/src/metrics/guarded_metrics.rs

Line 189 in e8f4a8a

.remove_label_values(&labels.each_ref().map(|s| s.as_str()))

might incorrectly deleted metrics from the inner Counter, thus, some inc_by() was actually lost.

Didn't fully understand the logic around uncollected_removed_labels...

[wenym1]

I am suspecting

risingwave/src/common/src/metrics/guarded_metrics.rs

Line 189 in e8f4a8a

.remove_label_values(&labels.each_ref().map(|s| s.as_str()))
might incorrectly deleted metrics from the inner Counter, thus, some inc_by() was actually lost.

Didn't fully understand the logic around uncollected_removed_labels...

The logic before using uncollected_removed_labels is, when the label is not used anywhere, we call remove_label_values on the unused label immediately, which may cause a loss of metrics because the label is removed before being collected.

After introducing uncollected_removed_labels, the unused label will be first added to this list, and on collect, we first call inner.collect() to collect metric of all labels, and then we call remove_label_values to really remove the label. In this way, we can avoid losing the metrics.