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

fix(monitored deploy): add deploymentId parameter #3255

Merged

Conversation

marchello2000
Copy link
Contributor

Monitors need an ability to correlate calls to their API.
This adds a deploymentId parameter to all requests (which is really the stageId for the createServerGroup stage)
This allows the monitor to correlate e.g. evaluateHealth @ 20% and evaluateHealth @ 50% to the same cluster
Especially if multiple shards are deployed in the same pipeline

Monitors need an ability to correlate calls to their API.
This adds a `deploymentId` parameter to all requests (which is really the `stageId` for the `createServerGroup` stage)
This allows the monitor to correlate e.g. `evaluateHealth @ 20%` and `evaluateHealth @ 50%` to the same cluster
Especially if multiple shards are deployed in the same pipeline
@@ -42,6 +81,7 @@ protected void fromStage(Stage stage) {
application = stage.getExecution().getApplication();
executionId = stage.getExecution().getId();
stageId = stage.getId();
deploymentId = stage.getParentStageId();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does stage id actually identify a deployment? Why not execution id?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The createServerGroup (parent) stage does - executionId is already there but it doesn't work when the same pipeline has multiple deploy stages. createServerGroup works because even if a deploy stage deploys N clusters there will be N createServerGroup stages and all steps will be their children

@gal-yardeni
Copy link
Contributor

Thanks for adding the comments @marchello2000 , super helpful!

@marchello2000 marchello2000 added the ready to merge Approved and ready for merge label Oct 25, 2019
@mergify mergify bot added the auto merged Merged automatically by a bot label Oct 25, 2019
@mergify mergify bot merged commit b6faf6f into spinnaker:master Oct 25, 2019
@marchello2000 marchello2000 deleted the mark/monitored_add_deployment_id branch November 1, 2019 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto merged Merged automatically by a bot ready to merge Approved and ready for merge target-release/1.17
Projects
None yet
4 participants