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

Improve integration with Metrics plugin #148

Merged

Conversation

jamesrobson-secondmind
Copy link
Contributor

@jamesrobson-secondmind jamesrobson-secondmind commented Nov 19, 2021

This improves the metrics generated:

  • Support the fallback causes logic, so only causes that will be reported for a build are counted
  • Adds a count of builds that failed without matching any causes, like the old visualisations did
  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@PayBas
Copy link

PayBas commented Nov 24, 2021

Nice 👍
Looking forward to this in the next release.

@jamesrobson-secondmind
Copy link
Contributor Author

jamesrobson-secondmind commented Nov 29, 2021

So I'm not sure what to do about testing this. There's currently no tests for the scan function or the fallback categories and this change is very simple, so I'm going to say no tests are needed unless someone objects to that.

@PayBas
Copy link

PayBas commented Nov 29, 2021

Well I'm currently using this PR in combination with my own #149 and while I am seeing:

# HELP jenkins_bfa_cause_no_matching_cause Generated from Dropwizard metric import (metric=jenkins_bfa.cause.no matching cause, type=com.codahale.metrics.Counter)
# TYPE jenkins_bfa_cause_no_matching_cause gauge
jenkins_bfa_cause_no_matching_cause 23.0

I'm not seeing the jenkins_bfa_category_### fallback categories. So either I integrated the 2 PRs incorrectly, or it's not working as intended.
Adding an automated test might help clear up this question ;)

@jamesrobson-secondmind
Copy link
Contributor Author

This has some tests now, can it be merged?

incCounters(UNKNOWNCAUSE);
}


Copy link

Choose a reason for hiding this comment

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

            if (!foundCauseList.isEmpty()) {
                incCounters(foundCauseList, PluginImpl.getInstance().isMetricSquashingEnabled());
            } else {
                incCounters(new ArrayList<>(Collections.singletonList(UNKNOWNCAUSE)),
                        PluginImpl.getInstance().isMetricSquashingEnabled());
            }

@PayBas
Copy link

PayBas commented Jan 31, 2022

Using this PR live for some time now. Looks good to me. But I'm not reviewer ;)

@rsandell rsandell merged commit 663a226 into jenkinsci:master Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants