Skip to content

fr-142 support jenkins.job.stage_completed#145

Merged
FlorianVeaux merged 4 commits into
jenkinsci:masterfrom
patelronak:master
Oct 16, 2020
Merged

fr-142 support jenkins.job.stage_completed#145
FlorianVeaux merged 4 commits into
jenkinsci:masterfrom
patelronak:master

Conversation

@patelronak

@patelronak patelronak commented Oct 14, 2020

Copy link
Copy Markdown
Contributor

Requirements for Contributing to this repository

What does this PR do?

#142

Description of the Change

Add additional metric when the stage completes
jenkins.job.stage_completed { stage_name, stage_depth, stage_parent, result }

Alternate Designs

Possible Drawbacks

Verification Process

I validated this manually.
I was not able to run mvn test successfully due to other failures.

Additional Notes

Release Notes

There is additional metric now
jenkins.job.stage_completed { stage_name, stage_depth, stage_parent, result }

Review checklist (to be filled by reviewers)

  • Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
  • PR should not have do-not-merge/ label attached.
  • If Applicable, issue must have kind/ and severity/ labels attached at least.

@patelronak

patelronak commented Oct 14, 2020

Copy link
Copy Markdown
Contributor Author

@FlorianVeaux I have created this PR and tested it locally. I was not able to run all mvn tests locally due to mocking errors. I would appreciate it if you can either use this as a base to take it forward or take it as is if all checks pass.

@patelronak

patelronak commented Oct 14, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

The error reported in the Check is
1.124 [id=119] SEVERE o.d.j.p.d.c.DatadogHttpClient#getInstance: Datadog API Key is not set properly
1.126 [id=119] SEVERE o.d.j.p.d.c.DogStatsDClient#getInstance: Datadog Target Port is not set properly
1.186 [id=119] INFO jenkins.model.Jenkins#cleanUp: Stopping Jenkins
1.372 [id=119] SEVERE o.d.j.p.d.c.DatadogHttpClient#post: Hmmm, your API key may be invalid. We received a 403 error.
1.372 [id=159] SEVERE o.d.j.p.d.c.DatadogHttpClient#post: Hmmm, your API key may be invalid. We received a 403 error.
1.373 [id=119] SEVERE o.d.j.p.datadog.DatadogUtilities#severe: An unexpected error occurred
1.373 [id=159] SEVERE o.d.j.p.datadog.DatadogUtilities#severe: An unexpected error occurred

which does not seem to be related to this PR?

@FlorianVeaux

FlorianVeaux commented Oct 15, 2020

Copy link
Copy Markdown
Contributor

The build is successful on my side locally and is ok as well on the CI (seems to be a windows related flake though but not related).

Thanks for the PR it looks good, one point though:
I believe submission should be using incrementCounter instead

datadog-plugin/src/main/java/org/datadog/jenkins/plugins/datadog/listeners/DatadogBuildListener.java

Line 270 in 685b75e

client.incrementCounter("jenkins.job.completed", hostname, tags);
.

Indeed that will make the metric a "count" on Datadog in-app (so the default time-aggregator will be set to sum). Also that will help if there are multiple stages with the same tags that finish at the same time. This way the value of the metric will accurately represent the total number of completed stages.

FlorianVeaux
FlorianVeaux reviewed Oct 15, 2020
View reviewed changes

@FlorianVeaux FlorianVeaux left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you also update the ReadMe to include this new metric?

Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/listeners/DatadogGraphListener.java Outdated
@FlorianVeaux FlorianVeaux added changelog/Added Added features results into a minor version bump community Community driven changes documentation Documentation related changes labels Oct 16, 2020
@FlorianVeaux

FlorianVeaux commented Oct 16, 2020

Copy link
Copy Markdown
Contributor

This is great thanks 🙇
The error on the Pull Request Labeler is because this job is supposed to add labels to PR automatically, but it can't work on forks as we can't give access to a github token to forks.

Merging as is and cutting a release later today

@FlorianVeaux
FlorianVeaux merged commit 283d731 into jenkinsci:master Oct 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@FlorianVeaux FlorianVeaux Awaiting requested review from FlorianVeaux

Assignees

No one assigned

Labels

changelog/Added Added features results into a minor version bump community Community driven changes documentation Documentation related changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@patelronak @FlorianVeaux