-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Make octopus deploy integration public #20392
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
Conversation
Hi! Adding a docs card for this one so we can review the assets before they're published. |
Review from AAraKKe is dismissed. Related teams and files:
- agent-integrations
- octopus_deploy/datadog_checks/octopus_deploy/data/conf.yaml.example
- octopus_deploy/metadata.csv
octopus_deploy/metadata.csv
Outdated
@@ -2,7 +2,7 @@ metric_name,metric_type,interval,unit_name,per_unit_name,description,orientation | |||
octopus_deploy.api.can_connect,gauge,,,,Whether or not the check can connect to the Octopus Deploy API.,-1,octopus_deploy,octopus_deploy api,, | |||
octopus_deploy.deployment.completed_time,gauge,,second,,Duration of deployment.,-1,octopus_deploy,octopus_deploy deploy dur,, | |||
octopus_deploy.deployment.count,gauge,,,,Number of deployments monitored.,-1,octopus_deploy,octopus_deploy deploy count,, | |||
octopus_deploy.deployment.executing,gauge,,second,,Whether or not the deployment is currently executing.,-1,octopus_deploy,octopus_deploy deploy executing,, | |||
octopus_deploy.deployment.executing,gauge,,,,Whether or not the deployment is currently executing.,-1,octopus_deploy,octopus_deploy deploy executing,, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a handful of lines in here where the unit_type
is second
but the description implies it's actually a boolean ("Whether or not..."). Can we do a pass here to confirm we're using the correct type on these lines?
octopus_deploy/metadata.csv
Outdated
@@ -2,7 +2,7 @@ metric_name,metric_type,interval,unit_name,per_unit_name,description,orientation | |||
octopus_deploy.api.can_connect,gauge,,,,Whether or not the check can connect to the Octopus Deploy API.,-1,octopus_deploy,octopus_deploy api,, | |||
octopus_deploy.deployment.completed_time,gauge,,second,,Duration of deployment.,-1,octopus_deploy,octopus_deploy deploy dur,, | |||
octopus_deploy.deployment.count,gauge,,,,Number of deployments monitored.,-1,octopus_deploy,octopus_deploy deploy count,, | |||
octopus_deploy.deployment.executing,gauge,,second,,Whether or not the deployment is currently executing.,-1,octopus_deploy,octopus_deploy deploy executing,, | |||
octopus_deploy.deployment.executing,gauge,,,,Whether or not the deployment is currently executing.,-1,octopus_deploy,octopus_deploy deploy executing,, | |||
octopus_deploy.deployment.executing_time,gauge,,second,,How long the deployment has been executing.,-1,octopus_deploy,octopus_deploy deploy dur,, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
octopus_deploy deploy dur
shortname is also used on line 3. Is that intentional?
Same deal for lines 7 and 8 - same shortname
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
octopus_deploy.api.can_connect,gauge,,,,Whether or not the check can connect to the Octopus Deploy API,-1,octopus_deploy,octopus_deploy api,, | ||
octopus_deploy.deployment.completed_time,gauge,,second,,Duration of deployment,-1,octopus_deploy,octopus_deploy deploy dur,, | ||
octopus_deploy.deployment.count,gauge,,,,Number of deployments monitored,-1,octopus_deploy,octopus_deploy deploy count,, | ||
octopus_deploy.deployment.executing,gauge,,,,Whether or not the deployment is currently executing,-1,octopus_deploy,octopus_deploy deploy executing,, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are a handful of lines in here where the unit_type
is second
but the description implies it's actually a boolean ("Whether or not..."). Can we do a pass here to confirm we're using the correct type on these lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated, thanks for finding that!
octopus_deploy/metadata.csv
Outdated
octopus_deploy.deployment.queued,gauge,,second,,Whether or not the deployment is currently in the queue,-1,octopus_deploy,octopus_deploy deploy queue,, | ||
octopus_deploy.deployment.queued_time,gauge,,second,,Time deployment was in queue,-1,octopus_deploy,octopus_deploy deploy queue,, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two have the same shortname. Is that intentional? Same with lines 3 and 6 - those also share a shortname
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, updated!
76f078e
to
95c1d20
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some suggested changes! Sorry if you got pinged a million times with my git nonsense (accidentally pushed a bunch of files on here so then had to revert). Anyway... take a look through everything and let me know what you think!
Looks good to me @domalessi, I'm going to get my team to review it! |
@sarah-witt Awesome! I'll go ahead and approve from the Documentation team so you're not blocked on me :) |
What does this PR do?
Makes the octopus deploy integration public and update some of the docs
Motivation
Review checklist (to be filled by reviewers)
qa/skip-qa
label if the PR doesn't need to be tested during QA.backport/<branch-name>
label to the PR and it will automatically open a backport PR once this one is merged