Validate HTTP connectivity for Agent traces port #464
Conversation
nikita-tkachenko-datadog
added
the
changelog/Added
nikita-tkachenko-datadog
force-pushed
the
nikita-tkachenko/configuration-refactoring-updated
branch
from
d20b98e to
b8e26e6
Compare
nikita-tkachenko-datadog
force-pushed
the
nikita-tkachenko/configuration-refactoring-updated
branch
2 times, most recently
from
0404aa5 to
0260867
Compare
nikita-tkachenko-datadog
force-pushed
the
nikita-tkachenko/agent-traces-validation
branch
from
df3e530 to
d70595b
Compare
| DatadogUtilities.severe(logger, ex, "Failed to create socket to host: " + host + ", port: " + port); | ||
| return ex.getMessage(); | ||
| try { | ||
| Set<String> endpoints = DatadogAgentClient.fetchAgentEndpoints(new HttpClient(10_000), agentHost, agentTraceCollectionPort); |
There was a problem hiding this comment.
Can we have a constant to name that 10_000?
Also, I'm seeing that this is the timeout. This means that if the plugin cannot communicate with the Datadog Agent, the request can take 10 seconds? This might be confusing for customers if there's no indication that the plugin is doing something under the hood.
If that's the case, could we limit the timeout to something lower (1 second? 2 seconds at most?) to provide quicker feedback.
There was a problem hiding this comment.
I've lowered the timeout to 2 seconds just to be on the safe side, and moved the value to a named constant.
| if (!endpoints.isEmpty()) { | ||
| return FormValidation.ok("Success!"); | ||
| } else { | ||
| return FormValidation.error("The agent returned empty endpoints list"); |
There was a problem hiding this comment.
I think we should use a more user-friendly error here to indicate what happened (Maybe something like Failed to communicate with host xxxxxx or similar?)
There was a problem hiding this comment.
Makes sense, updated the message
| DatadogUtilities.severe(logger, ex, "Failed to create socket to host: " + host + ", port: " + port); | ||
| return ex.getMessage(); | ||
| try { | ||
| Set<String> endpoints = DatadogAgentClient.fetchAgentEndpoints(new HttpClient(10_000), agentHost, agentTraceCollectionPort); |
There was a problem hiding this comment.
Is there a way that the plugin can send traces directly over TCP socket? Or this is not possible anymore?
There was a problem hiding this comment.
I'm not sure if this was ever possible, I haven't seen it neither in the plugin nor in the Java tracer.
nikita-tkachenko-datadog
deleted the
nikita-tkachenko/agent-traces-validation
branch
Requirements for Contributing to this repository
What does this PR do?
Improves connectivity check for Agent traces port.
Current check only verifies that a TCP socket can be opened using the Agent's host and configured traces port.
Traces are sent over HTTP, so it is possible that there is TCP connectivity, but not HTTP connectivity (for example, if there is an HTTP proxy in between Jenkins and the Agent).
The improved check verifies HTTP connectivity: it sends an HTTP request to the agent's
/infoendpoint. The check is considered successful if the info can be retrieved.Description of the Change
Alternate Designs
Possible Drawbacks
Verification Process
Additional Notes
Release Notes
Review checklist (to be filled by reviewers)
changelog/label attached. If applicable it should have thebackward-incompatiblelabel attached.do-not-merge/label attached.kind/andseverity/labels attached at least.