Skip to content

Commit

Permalink
fix: use ddcommon::parse_uri for input URL parsing
Browse files Browse the repository at this point in the history
Currently, we are not using the logic that involves parsing TCP, UDS, or Windows named pipe schemes, which breaks the detection of the connector, specifically for Windows named pipes.

This change introduces the use of ddcommon::parse_uri and also improves the error message returned when URL parsing fails.
  • Loading branch information
ganeshnj committed Feb 24, 2025
1 parent e47f5e9 commit 2f5706c
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 9 deletions.
2 changes: 1 addition & 1 deletion data-pipeline-ffi/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ impl From<TraceExporterError> for ExporterError {
AgentErrorKind::EmptyResponse => ExporterErrorCode::HttpEmptyBody,
},
TraceExporterError::Builder(e) => match e {
BuilderErrorKind::InvalidUri => ExporterErrorCode::InvalidUrl,
BuilderErrorKind::InvalidUri(_) => ExporterErrorCode::InvalidUrl,
BuilderErrorKind::InvalidTelemetryConfig => ExporterErrorCode::InvalidArgument,
},
TraceExporterError::Deserialization(_) => ExporterErrorCode::Serde,
Expand Down
25 changes: 20 additions & 5 deletions data-pipeline/src/trace_exporter/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ use rmp_serde::encode::Error as EncodeError;
use std::error::Error;
use std::fmt::{Debug, Display};

/// Represents different kinds of errors that can occur when interacting with the agent.
#[derive(Debug, PartialEq)]
pub enum AgentErrorKind {
/// Indicates that the agent returned an empty response.
EmptyResponse,
}

Expand All @@ -22,35 +24,48 @@ impl Display for AgentErrorKind {
}
}

/// Represents different kinds of errors that can occur during the builder process.
#[derive(Debug, PartialEq)]
pub enum BuilderErrorKind {
InvalidUri,
/// Represents an error when an invalid URI is provided.
/// The associated `String` contains underlying error message.
InvalidUri(String),
/// Indicates that the telemetry configuration is invalid.
InvalidTelemetryConfig,
}

impl Display for BuilderErrorKind {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
BuilderErrorKind::InvalidUri => write!(f, "Invalid URI"),
BuilderErrorKind::InvalidUri(msg) => write!(f, "Invalid URI: {}", msg),
BuilderErrorKind::InvalidTelemetryConfig => {
write!(f, "Invalid telemetry configuration")
}
}
}
}

/// Represents different kinds of network errors.
#[derive(Copy, Clone, Debug)]
pub enum NetworkErrorKind {
/// Indicates an error with the body of the request/response.
Body,
/// Indicates that the request was canceled.
Canceled,
/// Indicates that the connection was closed.
ConnectionClosed,
/// Indicates that the message is too large.
MessageTooLarge,
/// Indicates a parsing error.
Parse,
/// Indicates that the request timed out.
TimedOut,
/// Indicates an unknown error.
Unknown,
/// Indicates that the status code is incorrect.
WrongStatus,
}

/// Represents a network error, containing the kind of error and the source error.
#[derive(Debug)]
pub struct NetworkError {
kind: NetworkErrorKind,
Expand Down Expand Up @@ -134,8 +149,8 @@ impl From<EncodeError> for TraceExporterError {
}

impl From<hyper::http::uri::InvalidUri> for TraceExporterError {
fn from(_value: hyper::http::uri::InvalidUri) -> Self {
TraceExporterError::Builder(BuilderErrorKind::InvalidUri)
fn from(value: hyper::http::uri::InvalidUri) -> Self {
TraceExporterError::Builder(BuilderErrorKind::InvalidUri(value.to_string()))
}
}

Expand Down
20 changes: 17 additions & 3 deletions data-pipeline/src/trace_exporter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ use ddcommon::header::{
APPLICATION_MSGPACK_STR, DATADOG_SEND_REAL_HTTP_STATUS_STR, DATADOG_TRACE_COUNT_STR,
};
use ddcommon::tag::Tag;
use ddcommon::{connector, tag, Endpoint};
use ddcommon::{connector, parse_uri, tag, Endpoint};
use dogstatsd_client::{new, Client, DogStatsDAction};
use either::Either;
use error::BuilderErrorKind;
use hyper::body::HttpBody;
use hyper::http::uri::PathAndQuery;
use hyper::{header::CONTENT_TYPE, Body, Method, Uri};
Expand Down Expand Up @@ -782,6 +783,13 @@ pub struct TraceExporterBuilder {

impl TraceExporterBuilder {
/// Set url of the agent
/// The agent accepts TCP, UDS and Windows named pipe URLs.
/// TCP: http://<host>:<port>
/// eg: http://localhost:8126
/// UDS: unix://<path>
/// eg: unix://var/run/datadog/apm.socket
/// Windows named pipe: windows:\\.\pipe\<name>
/// eg: windows:\\.\pipe\datadog-apm
pub fn set_url(mut self, url: &str) -> Self {
self.url = Some(url.to_owned());
self
Expand Down Expand Up @@ -926,7 +934,10 @@ impl TraceExporterBuilder {
});

let base_url = self.url.as_deref().unwrap_or(DEFAULT_AGENT_URL);
let agent_url: hyper::Uri = base_url.parse()?;

let agent_url: hyper::Uri = parse_uri(base_url).map_err(|e: anyhow::Error| {
TraceExporterError::Builder(BuilderErrorKind::InvalidUri(e.to_string()))
})?;

let libdatadog_version = tag!("libdatadog_version", env!("CARGO_PKG_VERSION"));
let mut stats = StatsComputationStatus::Disabled;
Expand Down Expand Up @@ -1608,7 +1619,10 @@ mod tests {
_ => None,
};

assert_eq!(err.unwrap(), BuilderErrorKind::InvalidUri);
assert_eq!(
err.unwrap(),
BuilderErrorKind::InvalidUri("empty string".to_string())
);
}

#[test]
Expand Down

0 comments on commit 2f5706c

Please sign in to comment.