Refactor OpenTelemetry environment variable handling to use ray_constants

[wph95]

1. Remove direct environment variable access patterns

   • Replace all instances of os.getenv("RAY_enable_open_telemetry") == "1"
   • Standardize to use ray_constants.RAY_ENABLE_OPEN_TELEMETRY consistently throughout the codebase

2. Unify default value format for RAY_enable_open_telemetry

   • Standardize the default value to "true" | "false"
   • Previously, the codebase had mixed usage of "1" and "true", which is now unified

3. Backward compatibility maintained

   • Carefully verified that the existing RAY_ENABLE_OPEN_TELEMETRY constant properly handles both "1" and "true" values
   • This change will not introduce any breaking behavior
   • The env_bool helper function already supports both formats:

RAY_ENABLE_OPEN_TELEMETRY = env_bool("RAY_enable_open_telemetry", False)
def env_bool(key, default):
    if key in os.environ:
        return (
            True
            if os.environ[key].lower() == "true" or os.environ[key] == "1"
            else False
        )
    return default

---

Most of the current code uses: RAY_enable_open_telemetry: "1"

A smaller portion (not zero) uses: RAY_enable_open_telemetry: "true"

ray/python/ray/tests/test_metrics_agent.py

Line 497 in fe7ad00

"RAY_enable_open_telemetry": "true",

My personal preference is "true"—it’s concise and unambiguous. If it’s "1", I have to think/guess whether it means "true" or "false".

[edoakes]

@can-anyscale PTAL

[gemini-code-assist]

Code Review

This pull request refactors the handling of the RAY_enable_open_telemetry environment variable to consistently use the ray_constants.RAY_ENABLE_OPEN_TELEMETRY boolean constant. This is a great improvement as it centralizes the logic, ensures consistent behavior for both "1" and "true" values, and fixes latent bugs where the string "true" was not correctly handled. The changes are applied across test files and build configurations, standardizing on "true"/"false" values. The code is cleaner and more robust. I have one minor suggestion for further improvement.

[gemini-code-assist]

To improve maintainability and avoid magic strings, you can define the environment variable name as a local variable. This also allows for making the code more concise.

Suggested change

	if request.param:
	os.environ["RAY_enable_open_telemetry"] = "1"
	os.environ["RAY_enable_open_telemetry"] = "true"
	else:
	os.environ["RAY_enable_open_telemetry"] = "0"
	os.environ["RAY_enable_open_telemetry"] = "false"
	yield
	os.environ.pop("RAY_enable_open_telemetry", None)
	env_var = "RAY_enable_open_telemetry"
	os.environ[env_var] = "true" if request.param else "false"
	yield
	os.environ.pop(env_var, None)

[can-anyscale]

I think one of the bot's comments might be valid, this import is likely safe only for internal Ray components. Since ray.util.metrics is a public API, its behavior could be undefined if a program sets different values for RAY_ENABLE_OPEN_TELEMETRY.

import ray
from ray.util import metrics

os.environ["RAY_enable_open_telemetry"] = "true"
ray.init()

[cursor]

Bug: OpenTelemetry Configuration Breaks Backward Compatibility

The ray.util.metrics classes (e.g., Counter) now require RAY_enable_open_telemetry="true" to enable OpenTelemetry, breaking backward compatibility for users who set it to "1". This bypasses the RAY_ENABLE_OPEN_TELEMETRY constant, which is designed to handle both "1" and "true" values, contradicting the PR's backward compatibility claim.

 

[cursor]

Bug: Ray Metrics Module Breaks Backward Compatibility

The ray.util.metrics module now checks for RAY_enable_open_telemetry == "true", breaking backward compatibility for users who previously set it to "1". This causes the Counter class to incorrectly use CythonSum instead of CythonCount, which contradicts the PR's claim of maintaining backward compatibility.

 

[cursor]

Bug: Test Configuration Issue with Environment Variable

Replacing os.getenv("RAY_enable_open_telemetry") with the RAY_ENABLE_OPEN_TELEMETRY constant means test fixtures, like _setup_cluster_for_test, now use an import-time value. This can lead to stale enable_open_telemetry configurations if the environment variable changes after module import but before the fixture runs, affecting tests in test_metric_cardinality.py and test_metrics_agent.py.

Additional Locations (1)

• python/ray/tests/test_metrics_agent.py#L210-L211

 

[cursor]

Bug: Open Telemetry Configuration Not Dynamic

The RAY_ENABLE_OPEN_TELEMETRY constant is evaluated at module import time. This means if the RAY_enable_open_telemetry environment variable changes after module import but before a fixture or test uses it, the code will use a stale value. This regresses from previous dynamic environment checks.

Additional Locations (4)

• python/ray/tests/test_metric_cardinality.py#L39-L46
• python/ray/tests/test_metrics_agent.py#L325-L327
• python/ray/tests/test_metrics_agent.py#L24-L28
• python/ray/tests/test_metric_cardinality.py#L20-L21

 

[can-anyscale]

[cursor]

Bug: Inconsistent Env Evaluation Between Metrics and Ray Internal Measures

Inconsistent environment variable evaluation timing. The change uses env_bool("RAY_enable_open_telemetry", False) which evaluates the environment variable at runtime (when Counter.init is called), while other parts of the codebase use the RAY_ENABLE_OPEN_TELEMETRY constant which is evaluated at module import time. This creates undefined behavior when the environment variable is set after importing ray.util.metrics but before creating a Counter instance. Since ray.util.metrics is a public API, users could set the environment variable at any time, leading to inconsistent behavior between the metrics module and internal Ray components. The fix should import and use RAY_ENABLE_OPEN_TELEMETRY constant instead of calling env_bool() at runtime.

 

[can-anyscale]

it makes sense for test to evaluate at import time; in non-test code, it is consistently that the value will be evaluated at runtime

[can-anyscale]

it makes sense for test to evaluate at import time; in non-test code, it is consistently that the value will be evaluated at runtime

[can-anyscale]

Thank you for your contribution