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

Include userId in BuildData #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

LEDfan
Copy link

@LEDfan LEDfan commented Apr 28, 2021

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your master 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

Hi

For one of my projects, I gratefully use this plugin, however, it would be nice if the data in ElasticSearch included the username that triggered the build. I saw that there is a similar plugin that support datadog, that actually sends the userId. I copied their implementation into this plugin (and adapted the code a bit). In my test-cases this works very well. I added a test-case for all possible ways in which the userId can be determined.

I hope you can accept this PR, any feedback is welcome!

Implementation notes;

  • the userId is scm when the build was triggered by SCM
  • the userId is timer when the build was triggered by a timer
  • when the build was triggered by an upstream build, the userId of that
    build is used
  • in all other cases the userId is anonymous.

[1] https://github.com/jenkinsci/datadog-plugin/blob/master/src/main/java/org/datadog/jenkins/plugins/datadog/model/BuildData.java#L624


verify(mockBuild).getId();
verify(mockBuild, times(2)).getResult();
verify(mockBuild, times(2)).getParent();
verify(mockBuild, atLeast(2)).getParent();
Copy link
Author

Choose a reason for hiding this comment

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

I used atLeast instead of a fixed value here, because the amount of calls varies depending on the value returned by mockBuild.getCauses, therefore it depends on the exact contents of the test

Based on the similar implementation of the datadog plugin [1].
 - the userId is `scm` when the build was triggered by SCM
 - the userId is `timer` when the build was triggered by a timer
 - when the build was triggered by an upstream build, the userId of that
   build is used
 - in all other cases the userId is `anonymous`.

[1] https://github.com/jenkinsci/datadog-plugin/blob/master/src/main/java/org/datadog/jenkins/plugins/datadog/model/BuildData.java#L624
@LEDfan
Copy link
Author

LEDfan commented Nov 22, 2021

Hi @jakub-bochenski , could you be so kind to have a look at this PR? It would be very useful for us to have this included. Thanks in advance, but no worries if you do not have the time for it.

@PayBas
Copy link

PayBas commented Jan 25, 2022

Very interested in this as well. Hope to see this implemented/merged soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants