chore(datadog_metrics sink): support and migrate to the v2 series API endpoint
#18761
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| features: | ||
| - datadog-metrics-integration-tests | ||
|
|
||
| test_filter: '::datadog::metrics::integration_tests' | ||
|
|
||
| runner: | ||
| env: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,27 +42,58 @@ impl SinkBatchSettings for DatadogMetricsDefaultBatchSettings { | |
| const TIMEOUT_SECS: f64 = 2.0; | ||
| } | ||
|
|
||
| pub(super) const SERIES_V1_PATH: &str = "/api/v1/series"; | ||
| pub(super) const SERIES_V2_PATH: &str = "/api/v2/series"; | ||
| pub(super) const SKETCHES_PATH: &str = "/api/beta/sketches"; | ||
|
|
||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] | ||
| #[allow(dead_code)] | ||
| pub enum SeriesApiVersion { | ||
| V1, | ||
| V2, | ||
| } | ||
|
|
||
| impl SeriesApiVersion { | ||
| pub const fn get_path(self) -> &'static str { | ||
| match self { | ||
| Self::V1 => SERIES_V1_PATH, | ||
| Self::V2 => SERIES_V2_PATH, | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl Default for SeriesApiVersion { | ||
| fn default() -> Self { | ||
| Self::V2 | ||
| } | ||
| } | ||
|
|
||
|
Comment on lines
+45
to
+77
Contributor
There was a problem hiding this comment. What's the goal with all the configurable stuff here? Seems like the intent to move wholesale to the v2 API, since there's nothing exposed to allow staying on/switching back to the v1 API, at least from the user's perspective.
Contributor
Author
There was a problem hiding this comment. Yeah I was going to call this out when I finalized the PR, but I recall chatting with @jszwedko about this (the history of the convo is gone though) . I don't remember the details but I believe the conclusion was we wanted to keep the v1 logic if it wasn't too difficult to do so. We had discussed allowing the api version to be configurable somehow, but felt that wasn't necessary. So I think it might boil down to us wanting to be able to flip a switch to go back to V1 if there were issues with our rollout of V2. Another idea I had thrown out was having a non publicized backdoor essentially, that would allow a user to configure to use the v1. Its possible we could do that with the TLDR ... Its open for discussion 😆
Contributor
There was a problem hiding this comment. Yeah, I see including the fallback as a risk mitigation measure so that if it does break things, users have a workaround until we fix it. We can weigh the risk against the level of effort to maintain the v1 implementation though.
Contributor
Author
There was a problem hiding this comment.
In it's current state, users would have to edit the code and recompile in order to take advantage of that. But, my suggestion here:
, would allow us to give users a workaround that would just entail editing their config. If we did this, could tag it with a TODO and followup GH issue to remove the backdoor and the v1 support altogether, once it had enough soak time.
Contributor
There was a problem hiding this comment. If we'd like to expose this to the end user temporarily as we roll out this change, an unpublicized environment variable could be a good option. Something like
Contributor
Author
There was a problem hiding this comment. I can get behind a temporary env var.
To clarify- IMO I think we would only want to maintain the v1 implementation for a full release, maybe two. But after its clear there aren't issues, we'd remove it. In my view the maintenance burden of supporting the v1 implementation for a release or two should be very small if not zero.
Contributor
Author
There was a problem hiding this comment. I added the env var in the latest changes and made code comments regarding the temporary support of V1. Since this isn't a user-facing deprecation/change, I dont think we need to follow the formal DEPRECATION policy if I'm not mistaken
Contributor
Author
There was a problem hiding this comment. (Which I validated works by injecting it in the vdev env vars and seeing the debug logs from the integration tests)
Contributor
Author
There was a problem hiding this comment.
As discussed, I noted this in the DEPRECATIONS file but am holding off on a mention to the upgrade guide as its not a user facing change and is lowish risk |
||
| /// Various metric type-specific API types. | ||
| /// | ||
| /// Each of these corresponds to a specific request path when making a request to the agent API. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] | ||
| pub enum DatadogMetricsEndpoint { | ||
| Series(SeriesApiVersion), | ||
| Sketches, | ||
| } | ||
|
|
||
| impl DatadogMetricsEndpoint { | ||
| /// Gets the content type associated with the specific encoder for a given metric endpoint. | ||
| pub const fn content_type(self) -> &'static str { | ||
| match self { | ||
| Self::Series(SeriesApiVersion::V1) => "application/json", | ||
| Self::Sketches | Self::Series(SeriesApiVersion::V2) => "application/x-protobuf", | ||
| } | ||
| } | ||
|
|
||
| // Gets whether or not this is a series endpoint. | ||
| pub const fn is_series(self) -> bool { | ||
| matches!(self, Self::Series { .. }) | ||
| } | ||
|
|
||
| // Creates an instance of the `Series` variant with the default API version. | ||
| pub fn series() -> Self { | ||
| Self::Series(SeriesApiVersion::default()) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -84,7 +115,7 @@ impl DatadogMetricsEndpointConfiguration { | |
| /// Gets the URI for the given Datadog metrics endpoint. | ||
| pub fn get_uri_for_endpoint(&self, endpoint: DatadogMetricsEndpoint) -> Uri { | ||
| match endpoint { | ||
| DatadogMetricsEndpoint::Series { .. } => self.series_endpoint.clone(), | ||
| DatadogMetricsEndpoint::Sketches => self.sketches_endpoint.clone(), | ||
| } | ||
| } | ||
|
|
@@ -169,8 +200,8 @@ impl DatadogMetricsConfig { | |
| &self, | ||
| ) -> crate::Result<DatadogMetricsEndpointConfiguration> { | ||
| let base_uri = self.get_base_agent_endpoint(); | ||
| let series_endpoint = build_uri(&base_uri, SeriesApiVersion::default().get_path())?; | ||
| let sketches_endpoint = build_uri(&base_uri, SKETCHES_PATH)?; | ||
|
|
||
| Ok(DatadogMetricsEndpointConfiguration::new( | ||
| series_endpoint, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.