Skip to content
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

RUMM-895 Add single API to configure all endpoints #322

Merged

Conversation

ncreated
Copy link
Member

What and why?

📦 This PR introduces new APIs to configure Logs / Traces / RUM endpoint:

.set(endpoint: DatadogEndpoint) // .us | .eu | .gov
.set(customLogsEndpoint: URL)
.set(customTracesEndpoint: URL)
.set(customRUMEndpoint: URL)

It also marks old APIs as deprecated:

.set(logsEndpoint:)   // .us | .eu | .gov | .custom(...)
.set(tracesEndpoint:) // .us | .eu | .gov | .custom(...)
.set(rumEndpoint:)    // .us | .eu | .gov | .custom(...)

Old APIs were inconsistent: it made no sense to configure one feature for .us and another for .eu, because clientToken / RUM application ID are valid in only one server instance. Because default value was .us, this was leading to misconfigurations for clients who only set it to .eu for one product.

How?

Because we still need to support old APIs (while they are deprecated), following rules were implemented and tested:

  • if DatadogEndpoint is set, it overwrites values set for LogsEndpoint, TracesEndpoint and RUMEndpoint,
  • if custom<feature>Endpoint is set, it overwrites the value for that feature endpoint (including the value coming from DatadogEndpoint).

I also updated configuration tests in the way that makes it easier to remove old logic in the future.

Review checklist

  • Feature or bugfix MUST have appropriate tests (unit, integration)
  • Make sure each commit and the PR mention the Issue number or JIRA reference

@ncreated ncreated requested a review from a team as a code owner November 27, 2020 11:54
@ncreated ncreated self-assigned this Nov 27, 2020
@ncreated ncreated requested a review from a team as a code owner November 27, 2020 11:54
Copy link
Member

@mariusc83 mariusc83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ncreated ncreated merged commit 0594d47 into master Nov 27, 2020
@ncreated ncreated deleted the ncreated/RUMM-895-add-single-api-to-configure-all-endpoints branch November 27, 2020 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants