Add create-validation time to stream provisioning metric#1033
Merged
dhananjay-sawner merged 5 commits intoJun 23, 2026
Merged
Conversation
recordStreamProvisioningTime now adds the resource provider's create-validation time (datastream metadata system.createValidationTime.ms) on top of (now - system.creation.ms), so the stream provisioning histogram and the within/outside-SLA counters reflect validation plus creation time. Absent or malformed values are treated as 0, keeping behavior identical for streams created before this property existed. Also raise the default provisioning SLA threshold from 8 to 10 minutes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Compute createValidationTimeMs (defaulting to 0) and always add it, instead of conditionally adding inside the present-and-positive check. Logically identical, but the read-then-add-0-or-value flow is clearer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dhananjay-sawner
force-pushed
the
capture-create-validation-time
branch
3 times, most recently
from
97eeb3c to
6743449
Compare
Move the metadata read into a @VisibleForTesting static helper getCreateValidationTimeMs(Datastream). Add TestCoordinatorStandalone, a general home for cluster-free Coordinator unit tests (the integration test stays as TestCoordinator in datastream-server-restli). It covers present-and-positive (included), absent, non-positive, and malformed (all excluded as 0). The inline logic lived in a private void method that writes to an internally-constructed metrics object, so it was not directly assertable; a pure static helper is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dhananjay-sawner
force-pushed
the
capture-create-validation-time
branch
from
6743449 to
f152c5c
Compare
akshayrai
reviewed
Jun 23, 2026
| // Include the create-validation time in the stream provisioning time | ||
| streamProvisioningTimeMs += getCreateValidationTimeMs(ds); | ||
|
|
||
| _log.info("Total stream provisioning time for Datastream {} - {} ms", ds.getName(), streamProvisioningTimeMs); |
Collaborator
There was a problem hiding this comment.
might be good to log all the details - validation time, creation time and total?
Per review feedback, capture creation time and create-validation time separately and log all three when a stream reaches READY (creation + create-validation = total provisioning time). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dhananjay-sawner
force-pushed
the
capture-create-validation-time
branch
from
518913e to
32fe947
Compare
akshayrai
approved these changes
Jun 23, 2026
mittalprince
approved these changes
Jun 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
recordStreamProvisioningTimenow adds the create-validation time to the provisioning duration. The value comes from the datastream metadata propertysystem.createValidationTime.ms. Provisioning time becomes(now - system.creation.ms) + system.createValidationTime.ms, so thestreamProvisioningTimeMshistogram and the within/outside SLA counters reflect validation time plus creation time.system.createValidationTime.msis treated as 0, so streams created before the property existed keep computingnow - system.creation.msexactly as before.brooklin.server.coordinator.provisioningSlaThresholdMs) from 8 to 10 minutes.CREATE_VALIDATION_TIME_MStoDatastreamMetadataConstants.Testing Done
TestCoordinatorStandalonecoveringgetCreateValidationTimeMs: present-and-positive (included), absent, non-positive, and malformed (all excluded as 0).testProvisioningSlaThresholdConfigtoTestCoordinatorConfig: default is 10 minutes and the config override is honored../gradlew :datastream-server:testpasses.Notes for Reviewers
The new unit tests live in a new
TestCoordinatorStandaloneclass rather than the existingTestCoordinator, for following reasons:Coordinator.getCreateValidationTimeMs) is indatastream-server, but the existingTestCoordinatoris indatastream-server-restli. A unit test belongs in the same module as the code so./gradlew :datastream-server:testcovers it.TestCoordinatoris an integration test that stands up an embedded cluster. This is a pure, cluster-free unit test and does not belong in that test suite.datastream-serverhad no Coordinator unit-test class, and the name cannot beTestCoordinatorbecause that fully-qualified name is already used by the restli test.TestCoordinatorStandaloneis the general home for cluster-free Coordinator unit tests.