Skip to content

Commit 5e09108

Browse files
gibsondanDagster Devtools
authored andcommitted
Re-apply [telemetry] Fixes two bugs in the CI environment detection logic for telemetry (#20321)
Fixes two long-standing bugs in CI environment detection for telemetry. In particular, our detection of whether the `"CI"` env var was set was broken (it would never be detected), leading to large numbers of telemetry events being incorrectly marked as _not_ coming from CI. New unit tests ## Test Plan ## Changelog > If a changelog entry is required, replace this section with the desired entry. > > By default, no changelog entry will be produced for internal PRs. Co-authored-by: Sean Mackesey <s.mackesey@gmail.com> Synced-From-Internal GitOrigin-RevId: f2b4e11c72628ede9866faaabb1bf0c0c060d508
1 parent afc710b commit 5e09108

File tree

2 files changed

+67
-2
lines changed

2 files changed

+67
-2
lines changed

python_modules/dagster/dagster_tests/cli_tests/command_tests/test_telemetry.py

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
from dagster._core.workspace.load import load_workspace_process_context_from_yaml_paths
3434
from dagster._utils import pushd, script_relative_path
3535
from dagster_shared.telemetry import (
36+
KNOWN_CI_ENV_VAR_KEYS,
3637
cleanup_telemetry_logger,
3738
get_or_create_dir_from_dagster_home,
3839
get_telemetry_logger,
@@ -774,3 +775,67 @@ def test_user_id_consistent_across_dagster_homes():
774775

775776
# User IDs should be the same
776777
assert user_id_1 == user_id_2
778+
779+
780+
def test_is_known_ci_env_false_when_no_ci_env_vars(enabled_telemetry: Telemetry):
781+
"""Test that is_known_ci_env is False when no CI environment variables are set."""
782+
_, telemetry_caplog = enabled_telemetry
783+
784+
# Clear all CI-related env vars
785+
env_overrides: dict[str, Any] = {"CI": None}
786+
for key in KNOWN_CI_ENV_VAR_KEYS:
787+
env_overrides[key] = None
788+
789+
with environ(env_overrides):
790+
runner = CliRunner()
791+
with pushd(path_to_file("")):
792+
result = runner.invoke(
793+
job_execute_command,
794+
["-f", path_to_file("test_cli_commands.py"), "-a", "qux_job"],
795+
)
796+
assert result.exit_code == 0
797+
798+
for record in telemetry_caplog.records:
799+
message = json.loads(record.getMessage())
800+
assert message["is_known_ci_env"] is False
801+
802+
803+
def test_is_known_ci_env_true_when_generic_ci_env_var_set(enabled_telemetry: Telemetry):
804+
"""Test that is_known_ci_env is True when the generic CI env var is set."""
805+
_, telemetry_caplog = enabled_telemetry
806+
807+
# Clear specific CI env vars, but set generic CI
808+
env_overrides: dict[str, Any] = {}
809+
for key in KNOWN_CI_ENV_VAR_KEYS:
810+
env_overrides[key] = None
811+
812+
for val in ["true", "1", "yes"]:
813+
env_overrides["CI"] = val
814+
with environ(env_overrides):
815+
runner = CliRunner()
816+
with pushd(path_to_file("")):
817+
result = runner.invoke(
818+
job_execute_command,
819+
["-f", path_to_file("test_cli_commands.py"), "-a", "qux_job"],
820+
)
821+
assert result.exit_code == 0
822+
823+
for record in telemetry_caplog.records:
824+
message = json.loads(record.getMessage())
825+
assert message["is_known_ci_env"] is True
826+
827+
telemetry_caplog.clear()
828+
829+
830+
def test_known_ci_env_var_keys_contains_expected_entries():
831+
"""Verify KNOWN_CI_ENV_VAR_KEYS contains expected entries (catches string concatenation bugs)."""
832+
assert "CODEBUILD_BUILD_ID" in KNOWN_CI_ENV_VAR_KEYS
833+
assert "CIRCLECI" in KNOWN_CI_ENV_VAR_KEYS
834+
assert "GITHUB_ACTION" in KNOWN_CI_ENV_VAR_KEYS
835+
assert "GITLAB_CI" in KNOWN_CI_ENV_VAR_KEYS
836+
assert "JENKINS_URL" in KNOWN_CI_ENV_VAR_KEYS
837+
assert "BUILDKITE" in KNOWN_CI_ENV_VAR_KEYS
838+
assert "TRAVIS" in KNOWN_CI_ENV_VAR_KEYS
839+
assert "BITBUCKET_BUILD_NUMBER" in KNOWN_CI_ENV_VAR_KEYS
840+
# Ensure no accidentally concatenated strings
841+
assert "CODEBUILD_BUILD_IDCIRCLECI" not in KNOWN_CI_ENV_VAR_KEYS

python_modules/libraries/dagster-shared/dagster_shared/telemetry/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
"GITHUB_ACTION", # https://docs.github.com/en/actions/learn-github-actions/variables#default-environment-variables
2626
"BITBUCKET_BUILD_NUMBER", # https://support.atlassian.com/bitbucket-cloud/docs/variables-and-secrets/
2727
"JENKINS_URL", # https://www.jenkins.io/doc/book/pipeline/jenkinsfile/#using-environment-variables
28-
"CODEBUILD_BUILD_ID" # https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-env-vars.html
28+
"CODEBUILD_BUILD_ID", # https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-env-vars.html
2929
"CIRCLECI", # https://circleci.com/docs/variables/#built-in-environment-variables
3030
"TRAVIS", # https://docs.travis-ci.com/user/environment-variables/#default-environment-variables
3131
"BUILDKITE", # https://buildkite.com/docs/pipelines/environment-variables
@@ -39,7 +39,7 @@ def get_python_version() -> str:
3939

4040
def get_is_known_ci_env() -> bool:
4141
# Many CI tools will use `CI` key which lets us know for sure it's a CI env
42-
if os.environ.get("CI") is True:
42+
if os.environ.get("CI"):
4343
return True
4444

4545
# Otherwise looking for predefined env var keys of known CI tools

0 commit comments

Comments
 (0)