Keep env vars consistent #1197

TophrC-dd · 2024-02-15T19:13:53Z

What and why?

Currently datadog-ci currently looks between two variable for the datadog site.
DATADOG_SITE and DD_SITE except it doesn't always look at DD_SITE for some commands.
This can be frustrating especially where most documentation states to use DD_SITE which is heavily documented on the serverless documentation.
This PR is an attempt to be consistent in configuring the site so no matter which variable is used it will work across every command.

How?

I have made modifications to the helper api module that implements a new exported constant function called getDatadogSite which check both DATADOG_SITE and DD_SITE.
I also ensured that this order is kept if a customer preferred DATADOG_SITE that would be the first variable used.
However If neither environment variables are set then the default shall be DATADOG_SITE_US1 which is datadoghq.com.

I have also made updates to the commands to use this new export whereas before it would try to read either DATADOG_SITE or DD_SITE and sometimes both.

Review checklist

Because this make a change across all commands the current tests will validate that it is working properly.

Feature or bugfix MUST have appropriate tests (unit, integration)

…le across the application

src/commands/gate/utils.ts

src/helpers/api.ts

datadog-datadog-prod-us1 · 2024-02-16T13:31:10Z

Datadog Report

Branch report: keep-envvars-consistent
Commit report: 1524fb2
Test service: datadog-ci-tests

✅ 0 Failed, 1972 Passed, 0 Skipped, 2m 18.08s Total duration (37.75s time saved)

src/commands/dora/api.ts

src/commands/gate/utils.ts

src/helpers/api.ts

src/commands/git-metadata/upload.ts

src/helpers/api.ts

Co-authored-by: Corentin Girard <[email protected]>

Drarig29

LGTM

louiszawadzki

This introduces a tiny regression for mobile rum upload commands since we can either get the site from env variables or a configuration file.
More importantly it introduces different behaviours for different env variables.

louiszawadzki · 2024-02-28T09:48:48Z

src/commands/react-native/upload.ts

@@ -132,7 +133,6 @@ export class UploadCommand extends Command {
      this.config,
      {
        apiKey: process.env.DATADOG_API_KEY,
-        datadogSite: process.env.DATADOG_SITE,


This creates a small regression in the case where the site is both specified in the environment and the datadog-ci.json file with different values.

Before the order of priority used to be: env > config file > default value ('datadoghq.com')
Now since the site from env is put in the default value it will just be: config file > env.

It's only a tiny case but that also introduces a different logic for api key and site variables, which can make debugging harder.

I would suggest the following change here (we can either keep or remove the change above):

Suggested change

datadogSite: process.env.DATADOG_SITE,

datadogSite: process.env.DATADOG_SITE || process.env.DD_SITE,

louiszawadzki · 2024-02-28T09:49:34Z

src/commands/flutter-symbols/upload.ts

@@ -139,7 +140,6 @@ export class UploadCommand extends Command {
      this.config,
      {
        apiKey: process.env.DATADOG_API_KEY,
-        datadogSite: process.env.DATADOG_SITE,


This creates a small regression in the case where the site is both specified in the environment and the datadog-ci.json file with different values.

Before the order of priority used to be: env > config file > default value ('datadoghq.com')
Now since the site from env is put in the default value it will just be: config file > env.

It's only a tiny case but that also introduces a different logic for api key and site variables, which can make debugging harder.

I would suggest the following change here (we can either keep or remove the change above):

Suggested change

datadogSite: process.env.DATADOG_SITE,

datadogSite: process.env.DATADOG_SITE || process.env.DD_SITE,

louiszawadzki · 2024-02-28T09:49:40Z

src/commands/dsyms/upload.ts

@@ -77,7 +78,6 @@ export class UploadCommand extends Command {
      this.config,
      {
        apiKey: process.env.DATADOG_API_KEY,
-        datadogSite: process.env.DATADOG_SITE,


This creates a small regression in the case where the site is both specified in the environment and the datadog-ci.json file with different values.

Before the order of priority used to be: env > config file > default value ('datadoghq.com')
Now since the site from env is put in the default value it will just be: config file > env.

It's only a tiny case but that also introduces a different logic for api key and site variables, which can make debugging harder.

I would suggest the following change here (we can either keep or remove the change above):

Suggested change

datadogSite: process.env.DATADOG_SITE,

datadogSite: process.env.DATADOG_SITE || process.env.DD_SITE,

rpelliard · 2024-02-28T10:30:21Z

src/commands/git-metadata/upload.ts

@@ -216,7 +217,7 @@ export class UploadCommand extends Command {
  private getApiRequestBuilder(apiKey: string): RequestBuilder {
    return getRequestBuilder({
      apiKey,
-      baseUrl: 'https://' + apiHost,
+      baseUrl: `https://api.${getDatadogSite()}`,


nit: Small suggestion for consistency

Suggested change

baseUrl: `https://api.${getDatadogSite()}`,

baseUrl: `https://api.${this.config.datadogSite}`,

Could you also update the line 225 below to use this.config.datadogSite instead ? 🙏

dastrong

Consistency nit, could we update src/commands/sarif/utils.ts to use this util?

BenoitZugmeyer

PR looks good

TophrC-dd added 5 commits February 15, 2024 12:11

Ensuring that all of the api calls use a consistent environment varab…

7cc71cf

…le across the application

remove whitespacing

327b5d5

Linting

8ba1159

Missed a site environment variable

a8e0e27

remove extraline

82b833d

TophrC-dd requested review from a team as code owners February 15, 2024 19:13

TophrC-dd requested a review from juan-fernandez February 15, 2024 19:13

purple4reina reviewed Feb 15, 2024

View reviewed changes

src/commands/gate/utils.ts Outdated Show resolved Hide resolved

duncanista reviewed Feb 15, 2024

View reviewed changes

src/helpers/api.ts Outdated Show resolved Hide resolved

refactor code to be a function over a constant variable

330acab

Drarig29 reviewed Feb 16, 2024

View reviewed changes

src/commands/dora/api.ts Outdated Show resolved Hide resolved

src/commands/gate/utils.ts Outdated Show resolved Hide resolved

src/helpers/api.ts Outdated Show resolved Hide resolved

Addressing issues with the calling of getDatdogSite function

cabe1e2

Drarig29 changed the title ~~Keep envvars consistent~~ Keep env vars consistent Feb 16, 2024

Add unit tests

0bd8f60

TophrC-dd requested a review from a team as a code owner February 16, 2024 16:28

TophrC-dd requested a review from dastrong February 16, 2024 16:28

Fix gate unit tests

07f1d3f

Drarig29 reviewed Feb 16, 2024

View reviewed changes

src/commands/git-metadata/upload.ts Outdated Show resolved Hide resolved

src/helpers/api.ts Outdated Show resolved Hide resolved

TophrC-dd and others added 3 commits February 16, 2024 11:59

Fixing comment to match datadog language

52a4933

Update src/commands/git-metadata/upload.ts

4d1f14f

Co-authored-by: Corentin Girard <[email protected]>

Linting

1524fb2

Drarig29 approved these changes Feb 16, 2024

View reviewed changes

rpelliard approved these changes Feb 28, 2024

View reviewed changes

louiszawadzki suggested changes Feb 28, 2024

View reviewed changes

rpelliard reviewed Feb 28, 2024

View reviewed changes

dastrong approved these changes Feb 28, 2024

View reviewed changes

BenoitZugmeyer approved these changes Aug 8, 2024

View reviewed changes

Drarig29 added the topic:environment-variables label Oct 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Keep env vars consistent #1197

Keep env vars consistent #1197

TophrC-dd commented Feb 15, 2024 •

edited

Loading

datadog-datadog-prod-us1 bot commented Feb 16, 2024 •

edited

Loading

Drarig29 left a comment

louiszawadzki left a comment

louiszawadzki Feb 28, 2024

louiszawadzki Feb 28, 2024

louiszawadzki Feb 28, 2024

rpelliard Feb 28, 2024

dastrong left a comment

BenoitZugmeyer left a comment

	datadogSite: process.env.DATADOG_SITE,
	datadogSite: process.env.DATADOG_SITE \|\| process.env.DD_SITE,

	baseUrl: `https://api.${getDatadogSite()}`,
	baseUrl: `https://api.${this.config.datadogSite}`,

Keep env vars consistent #1197

Are you sure you want to change the base?

Keep env vars consistent #1197

Conversation

TophrC-dd commented Feb 15, 2024 • edited Loading

What and why?

How?

Review checklist

datadog-datadog-prod-us1 bot commented Feb 16, 2024 • edited Loading

Datadog Report

Drarig29 left a comment

Choose a reason for hiding this comment

louiszawadzki left a comment

Choose a reason for hiding this comment

louiszawadzki Feb 28, 2024

Choose a reason for hiding this comment

louiszawadzki Feb 28, 2024

Choose a reason for hiding this comment

louiszawadzki Feb 28, 2024

Choose a reason for hiding this comment

rpelliard Feb 28, 2024

Choose a reason for hiding this comment

dastrong left a comment

Choose a reason for hiding this comment

BenoitZugmeyer left a comment

Choose a reason for hiding this comment

TophrC-dd commented Feb 15, 2024 •

edited

Loading

datadog-datadog-prod-us1 bot commented Feb 16, 2024 •

edited

Loading