DataDog / datadog-ci

Use Datadog from your CI.
https://datadoghq.com
Apache License 2.0
127 stars 55 forks source link

Inconsistent use of Environment variables #1101

Open jrottenberg opened 11 months ago

jrottenberg commented 11 months ago

Bug description

Synthetics run-test is expecting DATADOG_* based variable, instead of DD_*

Usage in the readme is consistent https://github.com/DataDog/datadog-ci#container-image with the rest of datdog platform, but the application is still looking for DATADOG_ only:

https://github.com/DataDog/datadog-ci/blob/master/src/commands/synthetics/run-tests-command.ts#L205-L209

It seems to come from each individual command :

https://github.com/DataDog/datadog-ci/pull/1041 https://github.com/DataDog/datadog-ci/pull/1067

would be nice to fix it at the top level

Describe what you expected

Synthetics tests to start running but the process exits with an error :

Missing DATADOG_APP_KEY in your environment.

Steps to reproduce the issue

❯ docker run --rm -it -v $(pwd):/w -e DD_API_KEY=${MY_API_KEY} -e DD_APP_KEY=${MY_APP_KEY} datadog/ci synthetics run-tests -p ${MY_TEST}
Status: Downloaded newer image for datadog/ci:latest
Because `failOnCriticalErrors` is not set or disabled, the command will succeed. Use `failOnCriticalErrors: true` to make it fail instead.
Missing DATADOG_APP_KEY in your environment.

Additional context

Datadog CI - 2.23.0

Command

synthetics

Drarig29 commented 11 months ago

Hi @jrottenberg! Thanks for reporting, we created a card for that.

For now, you can use something like -e DATADOG_API_KEY=${DD_API_KEY}.

jrottenberg commented 11 months ago

That's right, in my gitlab-ci:

  script:
    # https://github.com/DataDog/datadog-ci/issues/1101
    - export DATADOG_API_KEY=${DD_API_KEY}
    - export DATADOG_APP_KEY=${DD_APP_KEY}
    - datadog-ci synthetics run-tests --public-id ${DD_SYNTHETICS_TEST} --jUnitReport junit.xml --failOnCriticalErrors --failOnTimeout
leggomuhgreggo commented 11 months ago

I'm curious to learn more about the switch away from DATADOG_* vars.

I know it used to be that you could use either DATADOG_* or DD_*

We'd been preferring the more verbose alternative, at my company, because it's easier to understand — and regret that we're pretty heavily coupled.

Would also suggest in future it might be better to consider these kinds of changes as breaking + major version bumps

Thanks!

Drarig29 commented 11 months ago

Hi @leggomuhgreggo!

We are trying to be closer to the datadog-agent which is arguably the most used, and uses DD_* vars. But recently in datadog-ci, we tried to support both.

Would also suggest in future it might be better to consider these kinds of changes as breaking + major version bumps

We always make sure we follow semver. However, the changes you mentioned were made on a beta command, so we allow ourselves such breaking changes.

leggomuhgreggo commented 11 months ago

Thanks for the informative response!

changes you mentioned were made on a beta command, so we allow ourselves such breaking changes.

Ah! That makes sense (regrets for the hasty assumption!)

We are trying to be closer to the datadog-agent

Oh dang — so datadog-agent never supported the DATADOG_* alternative then? Hmm — sticky situation!

(Whoa... I'm just realizing this might explain a couple long-standing issues with env tag propagation on some of our logs lol)

Anyway, I wish y'all well as you navigate these nuances. And thanks for the helpful context!