Skip to content
This repository was archived by the owner on Mar 24, 2025. It is now read-only.

Use processor/client module name as thrift rpc service. #1073

Closed
trevorriles wants to merge 3 commits into from
Closed

Use processor/client module name as thrift rpc service. #1073

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eba866f
Use processor/client module name as thrift rpc service.
Feb 21, 2025
927d677
Remove unecessary comment
Feb 21, 2025
67f7193
Apply formatting fixes
Feb 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion baseplate/clients/thrift.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ def _call_thrift_method(self: Any, *args: Any, **kwargs: Any) -> Any:
# this is technically incorrect, but we don't currently have a reliable way
# of getting the name of the service being called, so relying on the name of
# the client is the best we can do
rpc_service = self.namespace
rpc_service = self.client_cls.__module__
rpc_method = name

# RPC specific headers
Expand Down
2 changes: 1 addition & 1 deletion baseplate/server/thrift.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def handle(self, client_socket: socket.socket, address: Address) -> None:

otel_attributes = {
SpanAttributes.RPC_SYSTEM: "thrift",
SpanAttributes.RPC_SERVICE: self.processor.baseplate.service_name,
SpanAttributes.RPC_SERVICE: self.processor.__module__,
SpanAttributes.NET_HOST_IP: self.server_host,
SpanAttributes.NET_HOST_PORT: self.server_port,
}
Expand Down
14 changes: 9 additions & 5 deletions tests/integration/otel_thrift_tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ def example(self, context):
thrift_client_span,
{
SpanAttributes.RPC_SYSTEM: "thrift",
SpanAttributes.RPC_SERVICE: THRIFT_CLIENT_NAME,
SpanAttributes.RPC_SERVICE: "tests.integration.test_thrift.TestService",
SpanAttributes.RPC_METHOD: "example",
SpanAttributes.NET_PEER_NAME: "localhost",
SpanAttributes.NET_PEER_PORT: net_peer_port,
Expand Down Expand Up @@ -220,12 +220,14 @@ def example(self, context):
thrift_client_span.get_span_context().span_id,
)

self.assertEqual(thrift_client_span.name, "example_service/example")
self.assertEqual(
thrift_client_span.name, "tests.integration.test_thrift.TestService/example"
Copy link
Copy Markdown

@marcoferrer marcoferrer Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this contains the full python package name. To get something close to a "thrift" package, you'll need to only keep the last part of the package.

The assertion should be against test_thrift.TestService.

Copy link
Copy Markdown

@marcoferrer marcoferrer Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The python file name usually maps to the thrift schema name.

Copy link
Copy Markdown
Contributor Author

@trevorriles trevorriles Feb 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, is it always only a single depth package? I should be able to just split on . and take the last element.

)
self.assertSpanHasAttributes(
thrift_client_span,
{
SpanAttributes.RPC_SYSTEM: "thrift",
SpanAttributes.RPC_SERVICE: "example_service",
SpanAttributes.RPC_SERVICE: "tests.integration.test_thrift.TestService",
SpanAttributes.RPC_METHOD: "example",
SpanAttributes.NET_PEER_IP: "127.0.0.1",
SpanAttributes.NET_PEER_NAME: "localhost",
Expand All @@ -240,12 +242,14 @@ def example(self, context):
)
self.assertEqual(thrift_client_span.events[0].name, "message")

self.assertEqual(thrift_server_span.name, "tests.integration.otel_thrift_tests/example")
self.assertEqual(
thrift_server_span.name, "tests.integration.test_thrift.TestService/example"
)
self.assertSpanHasAttributes(
thrift_server_span,
{
SpanAttributes.RPC_SYSTEM: "thrift",
SpanAttributes.RPC_SERVICE: "tests.integration.otel_thrift_tests",
SpanAttributes.RPC_SERVICE: "tests.integration.test_thrift.TestService",
SpanAttributes.RPC_METHOD: "example",
SpanAttributes.NET_HOST_IP: "127.0.0.1",
SpanAttributes.NET_HOST_NAME: "localhost",
Expand Down