-
Notifications
You must be signed in to change notification settings - Fork 51
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
Add a link to Datadog to job details page #447
Add a link to Datadog to job details page #447
Conversation
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.
Requested changes as the link does not take into account the datacenter region. (Host is different between them)
public DatadogLinkAction(BuildData buildData) { // ci.pipeline.url%3A"https%3A%2F%2Fgoogle.com" | ||
String query = String.format("ci_level:pipeline @ci.pipeline.name:\"%s\" @ci.pipeline.number:%s", buildData.getJobName(), buildData.getBuildNumber("")); | ||
String urlEncodedQuery = URLEncoder.encode(query, StandardCharsets.UTF_8); | ||
this.url = String.format("https://app.datadoghq.com/ci/pipeline-executions?query=%s", urlEncodedQuery); |
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.
The host will be different depending on the datacenter and/or, if the org has custom subdomains.
E.g. For orgs in Europe, the url is https://app.datadoghq.eu/.........
, etc.
This is typically resolved by asking customer to provide a DD_SITE
environment variable (this does not cover custom subdomains, tho). For Jenkins, this does not seem the best approach, as it has its own config page.
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.
I know it's the case when using the intakes, but is it the same when logging in to the UI though? I thought for the app the URL is always the same, as in the static docs we have https://app.datadoghq.com
hardcoded.
I might be wrong though, will try asking someone who knows
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.
I have updated the link to take Datadog site into account.
If Agentless mode is used, the site can de derived from the plugin configuration.
If Agentful mode is used, we cannot reliably detect the site unfortunately (I've checked Agent endpoints, talked to the people from the Agent team - there seems to be no easy way at the moment). So for this case I have added a dropdown with the site to the config UI: if the users choose a site value there, the link will be shown, otherwise it won't.
9bf640e
to
490a0c7
Compare
5f3a210
to
d20b98e
Compare
d20b98e
to
b8e26e6
Compare
0404aa5
to
0260867
Compare
490a0c7
to
0277c0d
Compare
0277c0d
to
35bf37d
Compare
35bf37d
to
1b7f781
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.
Dropped some comments
public DatadogLinkAction(BuildData buildData, String siteName) { | ||
String query = String.format("ci_level:pipeline @ci.pipeline.name:\"%s\" @ci.pipeline.number:%s", buildData.getJobName(), buildData.getBuildNumber("")); | ||
String urlEncodedQuery = URLEncoder.encode(query, StandardCharsets.UTF_8); | ||
this.url = String.format("https://app.%s/ci/pipeline-executions?query=%s", siteName, urlEncodedQuery); |
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.
Notice that some orgs can have dedicated urls that are not the default app.%s
.
Using this link won't work for them. Should we add this limitation somewhere?
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.
Ahh, that is a good catch! I don't think there's a way to work around this, knowing only the site.
I asked around and did some tests of my own: the custom links indeed seem to replace the standard ones, not just be an alias in addition to them.
So what I ended up doing is adding a dedicated "Datadog App hostname" field to the plugin configuration.
The logic is as follows:
- if the field is configured, we use its value to construct the link URL
- if the field is empty, we try to infer the hostname using the intake URLs - this option will only work for agentless config
- if agentful connectivity is configured (and the Datadog App hostname is not provided), we will not show the link at all
<f:entry title="Datadog Site" field="site"> | ||
<f:select/> | ||
</f:entry> | ||
|
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.
This new entry is under DatadogAgentConfiguration
namespace. Is there any way to send data without using the agent?
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.
We only need the user to specify the site manually if they're reporting through the agent. For the agentless mode we can deduce the site automatically, which is why I've only added this setting to the agent section.
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.
This is not relevant anymore, now that I've added the dedicated field for Datadog App hostname (see the conversation above), so I reverted this addition.
public String getDisplayName() { | ||
return "View in Datadog"; | ||
} |
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.
Where this display is going to appear? Is it in the config section or in the pipeline detail?
I could not identify where it's used.
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 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.
Is there a possibility to disable this functionality? We use Datadog primarily for monitoring and not all of our developers have access to Datadog. This button is great for our Datadog-users, but it adds another button to the large list of actions on the Job Detail page.
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.
@sebastiaanspeck I have added an environment property (DATADOG_JENKINS_PLUGIN_SHOW_DATADOG_LINKS
) and a UI control to the plugin config to disable this: #495
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.
Dropped some comments
@@ -24,6 +24,7 @@ services: | |||
DATADOG_JENKINS_PLUGIN_TARGET_API_KEY: $JENKINS_PLUGIN_DATADOG_API_KEY | |||
DATADOG_JENKINS_PLUGIN_ENABLE_CI_VISIBILITY: true | |||
DATADOG_JENKINS_PLUGIN_CI_VISIBILITY_CI_INSTANCE_NAME: $JENKINS_PLUGIN_DATADOG_CI_INSTANCE_NAME | |||
DATADOG_JENKINS_PLUGIN_DATADOG_APP_HOSTNAME: $JENKINS_PLUGIN_DATADOG_APP_HOSTNAME |
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.
Should we document this new env var in this PR?
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.
Makes sense, added the env var (along with the UI and groovy-based config examples)
Requirements for Contributing to this repository
What does this PR do?
If CI Visibility is enabled, adds a "View in Datadog" link to the details page of every traced job.
Description of the Change
Alternate Designs
Possible Drawbacks
Verification Process
Tested manually.
Additional Notes
Release Notes
Review checklist (to be filled by reviewers)
changelog/
label attached. If applicable it should have thebackward-incompatible
label attached.do-not-merge/
label attached.kind/
andseverity/
labels attached at least.