fix: Redact sensitive data from OTEL traces and fix env var parsing #1553
Conversation
product-auto-label
bot
added
size: m
rajeevpodar
force-pushed
the
fix_hide_sensitive_query_keys
branch
from
7c4be24 to
91f04b2
Compare
chandra-siri
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
| val = os.environ.get(name, None) | ||
| if val is None: | ||
| return default | ||
| return str(val).strip().lower() in {"1", "true", "yes", "on"} |
There was a problem hiding this comment.
nit: coercing to str is not required. Just val.strip().lower() should work fine because output from os.environ.get is always str or None. If None, it's caught in line35 and default is returned.
There was a problem hiding this comment.
We need to add one more unit test . It should test that tracing are disabled when a user sets ENABLE_GCS_PYTHON_CLIENT_OTEL_TRACES to False (or any value other than {1, true, on, yes} )
ie negative unit test of test_enable_trace_yield_span
| full_url = client._connection.build_api_url(request.get("path")) | ||
| attr.update(_get_opentelemetry_attributes_from_url(full_url, strip_query=True)) | ||
| if "timeout" in request: | ||
| attr["connect_timeout,read_timeout"] = str(request.get("timeout")) |
There was a problem hiding this comment.
there's no need to change timeout section of the code and convert into str()
| "server.address": "testOtel.org", | ||
| "url.path": "/foo/bar/baz", | ||
| "url.scheme": "https", | ||
| "connect_timeout,read_timeout": str((100, 100)), |
There was a problem hiding this comment.
if we don't change timeout param in the production file, we need not change the same here in test file.
fix: Redact sensitive data from OTEL traces and fix env var parsing
Summary
This PR addresses two key improvements:
Redaction of Sensitive Data in OTEL Traces:
Robust Environment Variable Parsing:
Details
Motivation
Related Links
Please let me know if further changes or clarifications are needed.