Skip to content

Commit 56c2082

Browse files
committed
fix: address all PR review comments
- resource.py: Use raw IP for host.ip, add service.instance.id for <ip>-<pid> - distro/__init__.py: Use setdefault so user-provided values take precedence - hermes_agent/helpers.py: Handle schemeless URLs in _extract_server_info, return (None, None) when hostname is falsy; use agent_name for agent_id - vita/utils.py: Remove unused _infer_server_address function - claw-eval/wrappers.py: Change default fallback from 'openai' to 'unknown', add Google/Gemini provider detection, remove dead if-guard - agentscope/patch.py: Add comment explaining tool_type='function' assumption - dashscope/generation.py: Add comment about hardcoded server_address - tox-loongsuite.ini: Add loongsuite-distro test environment - test_resource.py/test_configurator.py: Update tests for new behavior Change-Id: I9128e7bb1db9bd10b1529a40c5f450e61dcc2aa8 Co-developed-by: Qoder <noreply@qoder.com>
1 parent 8906ae6 commit 56c2082

10 files changed

Lines changed: 58 additions & 27 deletions

File tree

instrumentation-loongsuite/loongsuite-instrumentation-agentscope/src/opentelemetry/instrumentation/agentscope/patch.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,8 @@ async def wrap_tool_call(wrapped, instance, args, kwargs, handler):
401401
matched_skill = _match_skill_for_tool(instance, tool_args)
402402

403403
# Create invocation object with all tool data
404+
# NOTE: tool_type is set to "function" as agentscope currently only
405+
# supports function-type tools. Update when other types are supported.
404406
invocation = ExecuteToolInvocation(
405407
tool_name=tool_name,
406408
tool_type="function",

instrumentation-loongsuite/loongsuite-instrumentation-claw-eval/src/opentelemetry/instrumentation/claw_eval/internal/wrappers.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ def _infer_provider_name(provider: Any) -> str:
7979
return "anthropic"
8080
if "dashscope" in cls_name:
8181
return "dashscope"
82+
if "gemini" in cls_name or "google" in cls_name:
83+
return "google"
8284
# Infer from model_id
8385
model_id = str(getattr(provider, "model_id", "") or "")
8486
if model_id.startswith(("gpt-", "o1-", "o3-", "chatgpt-")):
@@ -87,7 +89,9 @@ def _infer_provider_name(provider: Any) -> str:
8789
return "anthropic"
8890
if model_id.startswith(("qwen",)):
8991
return "dashscope"
90-
return "openai"
92+
if model_id.startswith(("gemini",)):
93+
return "google"
94+
return "unknown"
9195

9296

9397
# ---------------------------------------------------------------------------
@@ -544,8 +548,7 @@ def __call__(self, wrapped, instance, args, kwargs):
544548

545549
# Set gen_ai.provider.name (Required attribute)
546550
_provider_name = _infer_provider_name(provider)
547-
if _provider_name:
548-
span.set_attribute(GenAI.GEN_AI_PROVIDER_NAME, _provider_name)
551+
span.set_attribute(GenAI.GEN_AI_PROVIDER_NAME, _provider_name)
549552

550553
model_id = ""
551554
if provider is not None:

instrumentation-loongsuite/loongsuite-instrumentation-dashscope/src/opentelemetry/instrumentation/dashscope/utils/generation.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,8 @@ def _create_invocation_from_generation(
469469

470470
invocation = LLMInvocation(request_model=request_model)
471471
invocation.provider = "dashscope"
472+
# DashScope SDK always targets dashscope.aliyuncs.com; if custom endpoint
473+
# support is added in the future, extract from instance/env instead.
472474
invocation.server_address = "dashscope.aliyuncs.com"
473475
invocation.server_port = 443
474476
invocation.input_messages = _extract_input_messages(kwargs)

instrumentation-loongsuite/loongsuite-instrumentation-hermes-agent/src/opentelemetry/instrumentation/hermes_agent/helpers.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ def create_agent_invocation(
571571
invocation = InvokeAgentInvocation(
572572
provider=_HERMES_AGENT_SYSTEM,
573573
agent_name="Hermes",
574-
agent_id="hermes-agent",
574+
agent_id="Hermes",
575575
conversation_id=getattr(instance, "session_id", None),
576576
request_model=getattr(instance, "model", None),
577577
input_messages=[
@@ -616,11 +616,16 @@ def _extract_server_info(instance: Any) -> tuple[str | None, int | None]:
616616
if not base_url:
617617
return None, None
618618
base_url_str = str(base_url).rstrip("/")
619+
# Handle schemeless URLs by prepending "//" for urlparse.
620+
if "://" not in base_url_str:
621+
base_url_str = "//" + base_url_str
619622
try:
620623
from urllib.parse import urlparse # noqa: PLC0415
621624

622625
parsed = urlparse(base_url_str)
623626
address = parsed.hostname
627+
if not address:
628+
return None, None
624629
port = parsed.port
625630
if port is None:
626631
port = 443 if parsed.scheme == "https" else 80

instrumentation-loongsuite/loongsuite-instrumentation-vita/src/opentelemetry/instrumentation/vita/utils.py

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -149,20 +149,6 @@ def _infer_provider(model_name: str) -> str:
149149
return "unknown"
150150

151151

152-
def _infer_server_address(model: str) -> tuple[str | None, int | None]:
153-
"""Infer server address from model name for vita."""
154-
model_lower = model.lower() if model else ""
155-
if "qwen" in model_lower or "dashscope" in model_lower:
156-
return "dashscope.aliyuncs.com", 443
157-
if "gpt" in model_lower or "openai" in model_lower:
158-
return "api.openai.com", 443
159-
if "claude" in model_lower or "anthropic" in model_lower:
160-
return "api.anthropic.com", 443
161-
if "deepseek" in model_lower:
162-
return "api.deepseek.com", 443
163-
return None, None
164-
165-
166152
def _get_tool_definitions(
167153
tools: Any,
168154
) -> Optional[List[FunctionToolDefinition]]:

loongsuite-distro/src/loongsuite/distro/__init__.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@ def _configure(self, **kwargs: Any) -> None:
4040
resource_attributes = dict(
4141
kwargs.pop("resource_attributes", None) or {}
4242
)
43-
resource_attributes.update(
44-
LoongSuiteResourceDetector().detect().attributes
45-
)
43+
# Use setdefault so user-provided values take precedence over detector.
44+
detected = LoongSuiteResourceDetector().detect().attributes
45+
for k, v in detected.items():
46+
resource_attributes.setdefault(k, v)
4647
super()._configure(resource_attributes=resource_attributes, **kwargs)
4748

4849

loongsuite-distro/src/loongsuite/distro/resource.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
# Resource attribute keys contributed by LoongSuite.
2424
HOST_IP = "host.ip"
25+
SERVICE_INSTANCE_ID = "service.instance.id"
2526
GEN_AI_INSTRUMENTATION_SDK_NAME = "gen_ai.instrumentation.sdk.name"
2627

2728
# Fixed value identifying the GenAI instrumentation SDK shipped with LoongSuite.
@@ -67,14 +68,18 @@ class LoongSuiteResourceDetector(ResourceDetector):
6768
6869
Contributes the following attributes to the resource:
6970
70-
* ``host.ip`` formatted as ``<ip>-<pid>`` (e.g. ``127.0.0.1-1``).
71+
* ``host.ip`` — the raw host IP address (e.g. ``127.0.0.1``).
72+
* ``service.instance.id`` — ``<ip>-<pid>`` uniquely identifying the process
73+
instance (e.g. ``127.0.0.1-1``).
7174
* ``gen_ai.instrumentation.sdk.name`` set to ``loongsuite-genai-utils``.
7275
"""
7376

7477
def detect(self) -> Resource:
78+
host_ip = _get_host_ip()
7579
return Resource(
7680
{
77-
HOST_IP: _get_host_ip_with_pid(),
81+
HOST_IP: host_ip,
82+
SERVICE_INSTANCE_ID: f"{host_ip}-{os.getpid()}",
7883
GEN_AI_INSTRUMENTATION_SDK_NAME: _GEN_AI_INSTRUMENTATION_SDK_NAME_VALUE,
7984
}
8085
)

loongsuite-distro/tests/test_configurator.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
from loongsuite.distro.resource import (
2020
GEN_AI_INSTRUMENTATION_SDK_NAME,
2121
HOST_IP,
22+
SERVICE_INSTANCE_ID,
2223
)
2324

2425

@@ -30,20 +31,27 @@ def test_configure_injects_loongsuite_attributes(self, mock_init):
3031
mock_init.assert_called_once()
3132
resource_attributes = mock_init.call_args.kwargs["resource_attributes"]
3233
self.assertIn(HOST_IP, resource_attributes)
34+
self.assertIn(SERVICE_INSTANCE_ID, resource_attributes)
3335
self.assertEqual(
3436
resource_attributes[GEN_AI_INSTRUMENTATION_SDK_NAME],
3537
"loongsuite-genai-utils",
3638
)
3739

3840
@mock.patch("opentelemetry.sdk._configuration._initialize_components")
3941
def test_configure_preserves_existing_attributes(self, mock_init):
42+
"""User-provided values take precedence over detector values."""
4043
LoongSuiteConfigurator().configure(
41-
resource_attributes={"service.name": "my-service"}
44+
resource_attributes={
45+
"service.name": "my-service",
46+
HOST_IP: "custom-ip",
47+
}
4248
)
4349

4450
resource_attributes = mock_init.call_args.kwargs["resource_attributes"]
4551
self.assertEqual(resource_attributes["service.name"], "my-service")
46-
self.assertIn(HOST_IP, resource_attributes)
52+
# User-provided host.ip should NOT be overwritten by detector
53+
self.assertEqual(resource_attributes[HOST_IP], "custom-ip")
54+
self.assertIn(SERVICE_INSTANCE_ID, resource_attributes)
4755
self.assertIn(GEN_AI_INSTRUMENTATION_SDK_NAME, resource_attributes)
4856

4957
@mock.patch("opentelemetry.sdk._configuration._initialize_components")

loongsuite-distro/tests/test_resource.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
from loongsuite.distro.resource import (
1919
GEN_AI_INSTRUMENTATION_SDK_NAME,
2020
HOST_IP,
21+
SERVICE_INSTANCE_ID,
2122
LoongSuiteResourceDetector,
2223
_get_host_ip,
2324
_get_host_ip_with_pid,
@@ -37,8 +38,15 @@ def test_detect_returns_resource(self):
3738
def test_detect_contains_expected_keys(self):
3839
attributes = LoongSuiteResourceDetector().detect().attributes
3940
self.assertIn(HOST_IP, attributes)
41+
self.assertIn(SERVICE_INSTANCE_ID, attributes)
4042
self.assertIn(GEN_AI_INSTRUMENTATION_SDK_NAME, attributes)
4143

44+
def test_host_ip_is_raw_ip(self):
45+
"""host.ip should contain a raw IP address without PID suffix."""
46+
attributes = LoongSuiteResourceDetector().detect().attributes
47+
# Raw IP should not contain a dash-PID suffix
48+
self.assertNotIn("-", attributes[HOST_IP].rsplit(".", 1)[-1])
49+
4250
def test_gen_ai_instrumentation_sdk_name_value(self):
4351
attributes = LoongSuiteResourceDetector().detect().attributes
4452
self.assertEqual(
@@ -50,9 +58,12 @@ def test_gen_ai_instrumentation_sdk_name_value(self):
5058
@mock.patch(
5159
"loongsuite.distro.resource._get_host_ip", return_value="127.0.0.1"
5260
)
53-
def test_host_ip_format_is_ip_dash_pid(self, _mock_ip, _mock_pid):
61+
def test_service_instance_id_format_is_ip_dash_pid(
62+
self, _mock_ip, _mock_pid
63+
):
5464
attributes = LoongSuiteResourceDetector().detect().attributes
55-
self.assertEqual(attributes[HOST_IP], "127.0.0.1-1")
65+
self.assertEqual(attributes[SERVICE_INSTANCE_ID], "127.0.0.1-1")
66+
self.assertEqual(attributes[HOST_IP], "127.0.0.1")
5667

5768
@mock.patch("loongsuite.distro.resource.os.getpid", return_value=42)
5869
@mock.patch(

tox-loongsuite.ini

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,9 @@ envlist =
126126
py3{10,11,12,13}-test-loongsuite-instrumentation-wildtool
127127
lint-loongsuite-instrumentation-wildtool
128128

129+
; loongsuite-distro
130+
py3{9,10,11,12,13}-test-loongsuite-distro
131+
129132
[testenv]
130133
test_deps =
131134
opentelemetry-api@{env:CORE_REPO}\#egg=opentelemetry-api&subdirectory=opentelemetry-api
@@ -234,6 +237,9 @@ deps =
234237
wildtool: {[testenv]test_deps}
235238
wildtool: -r {toxinidir}/instrumentation-loongsuite/loongsuite-instrumentation-wildtool/test-requirements.txt
236239

240+
loongsuite-distro: {[testenv]test_deps}
241+
loongsuite-distro: {toxinidir}/loongsuite-distro
242+
237243
; FIXME: add coverage testing
238244
allowlist_externals =
239245
sh
@@ -334,6 +340,8 @@ commands =
334340
test-loongsuite-instrumentation-wildtool: pytest {toxinidir}/instrumentation-loongsuite/loongsuite-instrumentation-wildtool/tests {posargs}
335341
lint-loongsuite-instrumentation-wildtool: python -m ruff check {toxinidir}/instrumentation-loongsuite/loongsuite-instrumentation-wildtool
336342

343+
test-loongsuite-distro: pytest {toxinidir}/loongsuite-distro/tests {posargs}
344+
337345
; TODO: add coverage commands
338346
; coverage: {toxinidir}/scripts/coverage.sh
339347

0 commit comments

Comments
 (0)