Skip to content

[SPARK-59949] Record the operator.sdk controller execution histograms with sub-second precision - #949

Open
sankalpsthakur wants to merge 1 commit into
apache:mainfrom
sankalpsthakur:fix/SPARK-59949-controller-execution-nanos
Open

sankalpsthakur wants to merge 1 commit into
apache:mainfrom
sankalpsthakur:fix/SPARK-59949-controller-execution-nanos

Conversation

@sankalpsthakur

Copy link
Copy Markdown

What changes were proposed in this pull request?

This PR records the controller execution histograms of OperatorJosdkMetrics in nanoseconds instead of whole seconds.

  • timeControllerExecution measures the duration with Clock.nanoTime() and records it once per execution in both the cluster-wide and the namespaced histogram.
  • The histogram names end with nanos, e.g. sparkapplication.sparkappreconciler.reconcile.both.nanos and sparkapplication.sparkappreconciler.reconcile.failure.nanos, so PrometheusPullModelHandler exports them in seconds as operator_sdk_sparkapplication_sparkappreconciler_reconcile_both_seconds etc., like kubernetes_client_http_response_latency_seconds.
  • OperatorJosdkMetrics gets a package-private constructor taking a Clock, for tests, following SummingHistogram(Reservoir) of [SPARK-59935] Fix Prometheus _sum and quantiles of histograms and timers #922.
  • The migration guide gets an item for the renamed metrics.

Why are the changes needed?

timeControllerExecution updated its histograms with TimeUnit.MILLISECONDS.toSeconds(...), so the elapsed time was truncated to whole seconds and a reconciliation under one second, which most are, recorded 0. The quantiles of these histograms were mostly 0, and their Prometheus _sum (#922) undercounted. This is there since SPARK-48984 (0.1.0).

Does this PR introduce any user-facing change?

Yes. Compared to 1.0.0, the operator.sdk controller execution histograms are renamed with a nanos suffix, e.g. operator_sdk_sparkapplication_sparkappreconciler_reconcile_both_seconds instead of operator_sdk_sparkapplication_sparkappreconciler_reconcile_both in Prometheus, and their values are in seconds with sub-second precision instead of whole seconds. The migration guide is updated.

How was this patch tested?

Updated and new test cases:

  • OperatorJosdkMetricsTest.testTimeControllerExecution times the executions with a ManualClock and asserts that a 10 ms success and a 5 ms failure are recorded as 10000000 and 5000000 nanoseconds under the new names, instead of sleeping for a second.
  • PrometheusPullModelHandlerTest.testFormatMetricsSnapshotIncludesControllerExecutionHistogramInSeconds asserts that such a histogram is exported as ..._reconcile_both_seconds with 0.01 as its median and _sum.

Nothing was built or run locally; validation is the GitHub Actions run on this PR.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Claude Opus 5.5)

…ms with sub-second precision

Time the controller executions with Clock.nanoTime() and record the duration in nanoseconds in histograms whose names end with nanos, so that PrometheusPullModelHandler exports them in seconds. The histograms were updated with whole seconds, so an execution under one second recorded 0.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant