[metric] #3 Reset the declared_resources metric when at a new commit [restored] #1232

tiffanny29631 · 2024-05-22T21:38:55Z

The current OpenCensus library keeps a metric stream that is uniquely identified by commit alive even though no new values are issued.

This makes the Otel Collector metricstransform processor receives all time value when calculating the max and gives false output.

The temporary work around is to reset the stream of previous commit to 0 when parser sees new commit.

b/321875474

See the falsely closed PR #1219 for context.

tiffanny29631 · 2024-05-23T20:49:37Z

Context of using rate() instead of present_over_time() in PromQL: prometheus/prometheus#8255 the first proposal of adding a present_over_time() function to represent if a timeserie exists in the time frame.

In our case the error metrics (i.e. resource_fights_total) will always exist even though the value of it does not change when no resource fights are happening, which will make the present_over_time() always returning 1 http://screen/4wdqXrJqAUFP7PZ.

Using rate() instead can give us an idea whether a fight was recorded within the queried time frame - if no fights are recorded, the record rate should go back to 0.

e2e/nomostest/prometheus_metrics.go

pkg/metrics/otel.go

pkg/metrics/metrics.go

pkg/metrics/record.go

pkg/parse/run.go

pkg/declared/resources.go

e2e/testdata/otel-collector/otel-cm-full-gcm.yaml

tiffanny29631 · 2024-06-03T21:15:35Z

/retest-required

pkg/declared/resources.go

tiffanny29631 · 2024-06-05T22:13:55Z

/retest-required

karlkfi · 2024-06-06T18:19:58Z

pkg/declared/resources.go

+		metrics.RecordDeclaredResources(ctx, r.previousCommit, 0)
+	}
+
+	r.mutex.Lock()


just move these to the top of the method:
r.mutex.Lock()
defer r.mutex.Unlock()

Hmm yeah just did and tried with the same failing test, still not helping. TestDontDeleteAllNamespaces

Oh it's the order of deleteNS and update members. This time it should work.

…mit updates The current OpenCensus library keeps a metric stream that is uniquely identified by commit alive even though no new values are issued. This makes the Otel Collector metricstransform processor receives all time value when calculating the max and gives false output. The temporary work around is to reset the stream of previous commit to 0 when parser sees new commit. b/321875474

tiffanny29631 · 2024-06-06T21:35:29Z

/retest-required

karlkfi

/lgtm

google-oss-prow · 2024-06-07T17:48:36Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karlkfi

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [karlkfi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

…mit updates (GoogleContainerTools#1232) The current OpenCensus library keeps a metric stream that is uniquely identified by commit alive even though no new values are issued. This makes the Otel Collector metricstransform processor receives all time value when calculating the max and gives false output. The temporary work around is to reset the stream of previous commit to 0 when parser sees new commit. b/321875474

…when commit updates (GoogleContainerTools#1232)" This reverts commit 6163281.

The fix GoogleContainerTools#1232 only affects declared_resources from Config Sync. The metric from RG controller may take longer to be ready. Skip checking for those target values for now.

The fix #1232 only affects declared_resources from Config Sync. The metric from RG controller may take longer to be ready. Skip checking for those target values for now.

…mit updates (GoogleContainerTools#1232) The current OpenCensus library keeps a metric stream that is uniquely identified by commit alive even though no new values are issued. This makes the Otel Collector metricstransform processor receives all time value when calculating the max and gives false output. The temporary work around is to reset the stream of previous commit to 0 when parser sees new commit. b/321875474

The fix GoogleContainerTools#1232 only affects declared_resources from Config Sync. The metric from RG controller may take longer to be ready. Skip checking for those target values for now.

…ix (#1232) (#1254) * [metric] #3 Reset declared_resource to 0 when commit updates (#1232) The current OpenCensus library keeps a metric stream that is uniquely identified by commit alive even though no new values are issued. This makes the Otel Collector metricstransform processor receives all time value when calculating the max and gives false output. The temporary work around is to reset the stream of previous commit to 0 when parser sees new commit. b/321875474 * Use default timeout for metric value check (#1264) * test: Only check declared_resources metric (#1268) The fix #1232 only affects declared_resources from Config Sync. The metric from RG controller may take longer to be ready. Skip checking for those target values for now.

google-oss-prow bot requested review from karlkfi and sdowell May 22, 2024 21:38

google-oss-prow bot added the size/XL label May 22, 2024

tiffanny29631 force-pushed the opencensus-otel branch 2 times, most recently from a770ea3 to 55a2b91 Compare May 23, 2024 18:48