Skip to content
This repository was archived by the owner on Jan 23, 2020. It is now read-only.

Send all metrics w/o DogstatsD#169

Merged
gzussa merged 30 commits into
masterfrom
davidb/threadlocal
Nov 27, 2019
Merged

Send all metrics w/o DogstatsD#169
gzussa merged 30 commits into
masterfrom
davidb/threadlocal

Conversation

@dabcoder

@dabcoder dabcoder commented Nov 14, 2019

Copy link
Copy Markdown
  • Get rid of the DogStatsD dependency by creating a thread safe counter cache
    • Use a ConcurrentMap to make sure that increments are thread safe - prevent concurrency issue - I may have been over protective with my implementation
    • Use a ReentrantLock to make sure we don't flush and increment the counter cache at the same time (across threads).
    • Use the singleton pattern to only have one instance of the cache in the JVM (also thread safe)
    • Counter are submitted as COUNT with an interval set to 10 s (recommended interval)
  • It also adds jenkins.job.started and jenkins.scm.checkout metrics.
  • It increases test coverage!
  • Wrapping all extensions in global try catch statements so build don't fail because of the plugin - see issue discussed in Provide option to post to local dogstatsd rather than going directly to the API #34 (however, in this PR we are removing DogStatsD dependency)
  • Changed the DatadogHttpClient into another singleton - thread safe - Hence we don't have lease logic anymore from the descriptor.

@dabcoder
dabcoder requested review from gzussa and nmuesch November 14, 2019 17:00

@gzussa gzussa left a comment

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.

Didn't check tests at this point. Just the core implementation. Good clean up 👍

Comment thread pom.xml Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/DatadogMetricCounter.java Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/DatadogMetricCounter.java Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/DatadogMetricCounter.java Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/DatadogMetricCounter.java Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/model/LocalCacheCounters.java Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/model/LocalCacheCounters.java Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/model/LocalCacheCounters.java Outdated
@dabcoder
dabcoder marked this pull request as ready for review November 18, 2019 12:06
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/DatadogBuildListener.java Outdated

@gzussa gzussa left a comment

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.

Great. Made comment to push this further.

  • Let's manually test this to make sure it work. I have a doubt that resetting the cache before submitting metrics may be a problem.
  • Also we should go deeper testing mttr, cycleTime, leadTime metrics. Each deserve their own tests.

Comment thread pom.xml Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/DatadogBuildListener.java Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/DatadogBuildListener.java Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/DatadogBuildListener.java Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/DatadogBuildListener.java Outdated
Comment thread src/test/java/org/datadog/jenkins/plugins/datadog/DatadogBuildListenerTest.java Outdated
Comment thread src/test/java/org/datadog/jenkins/plugins/datadog/DatadogBuildListenerTest.java Outdated
Comment thread src/test/java/org/datadog/jenkins/plugins/datadog/DatadogBuildListenerTest.java Outdated
@gzussa
gzussa requested a review from a team November 22, 2019 16:35
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/clients/DatadogHttpClient.java Outdated

@gzussa gzussa left a comment

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.

Approved 😆

This was referenced Nov 27, 2019
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/clients/DatadogHttpClient.java Outdated
Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/clients/DatadogHttpClient.java Outdated
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants