Skip to content

Commit 91dad5c

Browse files
CopilotnikhilNava
andcommitted
Fix lint errors and address code review feedback
- Remove trailing whitespace (auto-formatted with ruff) - Add validation for malformed URLs like "http:8080" (missing slashes) - Update comment to clarify "domain with optional port" instead of "just a domain" - Add tests for malformed URL edge cases - All 30 tests passing Co-authored-by: nikhilNava <[email protected]>
1 parent cac6bb3 commit 91dad5c

File tree

3 files changed

+26
-16
lines changed

3 files changed

+26
-16
lines changed

libraries/microsoft-agents-a365-observability-core/microsoft_agents_a365/observability/core/exporters/agent365_exporter.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,13 @@ def export(self, spans: Sequence[ReadableSpan]) -> SpanExportResult:
9595
else:
9696
discovery = PowerPlatformApiDiscovery(self._cluster_category)
9797
endpoint = discovery.get_tenant_island_cluster_endpoint(tenant_id)
98-
98+
9999
endpoint_path = (
100100
f"/maven/agent365/service/agents/{agent_id}/traces"
101101
if self._use_s2s_endpoint
102102
else f"/maven/agent365/agents/{agent_id}/traces"
103103
)
104-
104+
105105
# Construct URL - if endpoint has a scheme (http:// or https://), use it as-is
106106
# Otherwise, prepend https://
107107
# Note: Check for "://" to distinguish between real protocols and domain:port format

libraries/microsoft-agents-a365-observability-core/microsoft_agents_a365/observability/core/exporters/utils.py

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ def get_validated_domain_override() -> str | None:
157157
# Validate that it's a valid URL
158158
try:
159159
parsed = urlparse(domain_override)
160-
160+
161161
# If scheme is present and looks like a protocol (contains //)
162162
# Note: We check for "://" because urlparse treats "example.com:8080" as having
163163
# scheme="example.com", but this is actually a domain with port, not a protocol.
@@ -171,14 +171,18 @@ def get_validated_domain_override() -> str | None:
171171
return None
172172
# Must have a netloc (hostname) when scheme is present
173173
if not parsed.netloc:
174+
logger.warning(f"Invalid domain override '{domain_override}': missing hostname")
175+
return None
176+
else:
177+
# If no scheme with ://, it should be a domain with optional port (no path)
178+
# Note: domain can contain : for port (e.g., example.com:8080)
179+
# Reject malformed URLs like "http:8080" that look like protocols but aren't
180+
if domain_override.startswith(("http:", "https:")) and "://" not in domain_override:
174181
logger.warning(
175182
f"Invalid domain override '{domain_override}': "
176-
"missing hostname"
183+
"malformed URL - protocol requires '://'"
177184
)
178185
return None
179-
else:
180-
# If no scheme with ://, it should be just a domain (no path)
181-
# Note: domain can contain : for port (e.g., example.com:8080)
182186
if "/" in domain_override:
183187
logger.warning(
184188
f"Invalid domain override '{domain_override}': "

tests/observability/core/exporters/test_utils.py

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,7 @@ def test_accepts_valid_domain_with_port(self):
101101

102102
def test_accepts_valid_https_url(self):
103103
"""Test that function accepts a valid URL with https protocol."""
104-
with patch.dict(
105-
os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "https://example.com"}
106-
):
104+
with patch.dict(os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "https://example.com"}):
107105
result = get_validated_domain_override()
108106
self.assertEqual(result, "https://example.com")
109107

@@ -123,17 +121,13 @@ def test_accepts_valid_http_url_with_port(self):
123121

124122
def test_rejects_invalid_protocol(self):
125123
"""Test that function rejects URLs with invalid protocols (not http/https)."""
126-
with patch.dict(
127-
os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "ftp://example.com"}
128-
):
124+
with patch.dict(os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "ftp://example.com"}):
129125
result = get_validated_domain_override()
130126
self.assertIsNone(result)
131127

132128
def test_rejects_domain_with_path(self):
133129
"""Test that function rejects domain-only format with path separator."""
134-
with patch.dict(
135-
os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "example.com/path"}
136-
):
130+
with patch.dict(os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "example.com/path"}):
137131
result = get_validated_domain_override()
138132
self.assertIsNone(result)
139133

@@ -143,6 +137,18 @@ def test_rejects_protocol_without_hostname(self):
143137
result = get_validated_domain_override()
144138
self.assertIsNone(result)
145139

140+
def test_rejects_malformed_url_http_colon(self):
141+
"""Test that function rejects malformed URLs like 'http:8080' (missing slashes)."""
142+
with patch.dict(os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "http:8080"}):
143+
result = get_validated_domain_override()
144+
self.assertIsNone(result)
145+
146+
def test_rejects_malformed_url_https_colon(self):
147+
"""Test that function rejects malformed URLs like 'https:443' (missing slashes)."""
148+
with patch.dict(os.environ, {"A365_OBSERVABILITY_DOMAIN_OVERRIDE": "https:443"}):
149+
result = get_validated_domain_override()
150+
self.assertIsNone(result)
151+
146152

147153
if __name__ == "__main__":
148154
unittest.main()

0 commit comments

Comments
 (0)