Send webhooks via the Agent EVP Proxy when supported by albertvaka · Pull Request #316 · jenkinsci/datadog-plugin

albertvaka · 2022-10-31T18:41:58Z

What does this PR do?

Checks whether the Agent supports the EVP Proxy endpoint.
If it does, it generates webhooks instead of traces and sends them using the EVP Proxy.
If it doesn't, continues to generate traces and send them via APM.

I also added tests for the whole webhooks logic, which didn't have any. They are mostly copies of the tests for the traces logic.

Description of the Change

Adds a fetchAgentSupportedEndpoints() method to DatadogAgentClient that queries the /info Agent API.
Makes the postWebhook() method part of the abstract DatadogClient so that DatadogWebhookBuildLogic and DatadogWebhookPipelineLogic can call it on either a DatadogAgentClient or a DatadogHttpClient instance.
Implements postWebhook() in DatadogAgentClient. It sends webhook-type payloads using the Agent EVP Proxy.
The public methods in DatadogTraceBuildLogic and DatadogWebhookBuildLogic are now @Overrides of abstract methods of their parent DatadogBaseBuildLogic so DatadogAgentClient can use either implementation (depending on the output of fetchAgentSupportedEndpoints). Same for Datadog*PipelineLogic.

Verification Process

Test against an Agent that supports EVP Proxy and one that doesn't

Open Questions

If Jenkins starts before the Agent, then the EVP Proxy support will be disabled because we won't be able to query the /info API when initializing the DatadogAgentClient. Is this okay?
- ~~I've added 3 retries at startup time to wait for the Agent.~~
- I've made checking the /info endpoint lazy. See my comment below.

albertvaka · 2022-11-15T11:23:22Z

I've changed the detection of the EVP Proxy support to be lazy, so it is checked when it's first needed and the result is cached for 1 hour.

This way we don't block at boot time and in the worst case scenario we add a 10 second delay (the timeout of the /info request) once every hour. Most times, the Agent will respond in a few milliseconds or not respond at all (ie: if it's not there or doesn't support the /info API) which will not block.

This also lets us support Agent upgrades without having to restart Jenkins.

And use Mockito.spy to simplify tests

drodriguezhdez · 2022-11-24T09:57:47Z


+    protected boolean checkEvpProxySupportAndUpdateLogic() {
+        if (evpProxySupported) {
+            return true; // Once we have seen an Agent that supports EVP Proxy, we never check again.


Could we have the case of customers downgrading the DD Agent?

I decided to not support that case so we don't have to always query /info.

I guess dropping a log message or pointing this case on docs would be great.

drodriguezhdez · 2022-11-24T10:00:26Z

+                    traceBuildLogic = new DatadogWebhookBuildLogic(this);
+                    tracePipelineLogic = new DatadogWebhookPipelineLogic(this);


Could we have race conditions here? Like, the logic started using the version with APM, and then, the logic changes, and it changes to WebHooks? Do we need to protect this case?

If some spans get sent as traces and others as webhooks, nothing bad happens. I just checked it and the backend even displays them as the same pipeline because of the same trace id!

drodriguezhdez

LGTM

drodriguezhdez · 2022-11-28T14:46:42Z


+    protected boolean checkEvpProxySupportAndUpdateLogic() {
+        if (evpProxySupported) {
+            return true; // Once we have seen an Agent that supports EVP Proxy, we never check again.


I guess dropping a log message or pointing this case on docs would be great.

drodriguezhdez · 2022-11-28T14:47:15Z

+                    traceBuildLogic = new DatadogWebhookBuildLogic(this);
+                    tracePipelineLogic = new DatadogWebhookPipelineLogic(this);


albertvaka force-pushed the albertvaka/webhooks-via-agent branch from 916b09e to cc56d74 Compare November 2, 2022 23:10

albertvaka marked this pull request as ready for review November 2, 2022 23:10

albertvaka force-pushed the albertvaka/webhooks-via-agent branch 2 times, most recently from ded1755 to 3f00968 Compare November 3, 2022 11:52

sarah-witt reviewed Nov 7, 2022

View reviewed changes

albertvaka force-pushed the albertvaka/webhooks-via-agent branch from 9836328 to 77679f6 Compare November 8, 2022 16:38

drodriguezhdez reviewed Nov 11, 2022

View reviewed changes

Comment thread src/main/java/org/datadog/jenkins/plugins/datadog/clients/DatadogAgentClient.java Outdated

albertvaka force-pushed the albertvaka/webhooks-via-agent branch 8 times, most recently from b53f7d8 to 2386e37 Compare November 17, 2022 10:41

albertvaka and others added 9 commits November 18, 2022 13:54

Send webhooks via the Agent EVP Proxy when supported

f85f889

Add tests

72d065f

Wait up to 30 seconds for the Datadog Agent to be available at startup

e8cdd66

typo

b1029a6

Bump evp proxy requirement to v3, since v1 won't forward headers

8667239

Fixes as per code review

191d27a

Calculate stage breakdown for webhooks also

8e54a09

Log if the EVP Proxy is supported at info level

95bd77e

Remove retries, add lazy initialization

576f959

And use Mockito.spy to simplify tests

albertvaka force-pushed the albertvaka/webhooks-via-agent branch from 2386e37 to 576f959 Compare November 18, 2022 12:54

albertvaka added 2 commits November 18, 2022 14:44

Added debug log

36719cc

Log the right way

e00b744

albertvaka force-pushed the albertvaka/webhooks-via-agent branch from 19fa753 to a0d6d5f Compare November 18, 2022 16:11

Get the parent span id from the spancontext

612116c

albertvaka force-pushed the albertvaka/webhooks-via-agent branch from a0d6d5f to 612116c Compare November 18, 2022 16:13

Add tests for the webhooks logic

c265e86

albertvaka force-pushed the albertvaka/webhooks-via-agent branch from 8f8b206 to c265e86 Compare November 24, 2022 00:10

drodriguezhdez reviewed Nov 24, 2022

View reviewed changes

Lower log level

8d6c013

drodriguezhdez previously approved these changes Nov 28, 2022

View reviewed changes

Improve logging

a4c3158

albertvaka dismissed drodriguezhdez’s stale review via a4c3158 November 28, 2022 15:55

drodriguezhdez approved these changes Nov 28, 2022

View reviewed changes

albertvaka merged commit a7f3b94 into master Nov 28, 2022

albertvaka deleted the albertvaka/webhooks-via-agent branch November 28, 2022 16:11

albertvaka added the changelog/Added Added features results into a minor version bump label Dec 13, 2022

		traceBuildLogic = new DatadogWebhookBuildLogic(this);
		tracePipelineLogic = new DatadogWebhookPipelineLogic(this);

Uh oh!

Conversation

albertvaka commented Oct 31, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Description of the Change

Verification Process

Open Questions

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

albertvaka commented Nov 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drodriguezhdez left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

albertvaka commented Oct 31, 2022 •

edited

Loading

albertvaka commented Nov 15, 2022 •

edited

Loading