Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(profiling): Write thread id to transactions table #6138

Merged
merged 3 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion snuba/datasets/processors/transactions_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,13 @@ def _sanitize_contexts(
transaction_ctx.pop("trace_id", None)
transaction_ctx.pop("span_id", None)

# Only top level scalar values within a context are written to the table. `data` is
# always a dict, so pop it from the context and move some values into the top level.
transaction_data = transaction_ctx.pop("data", {})
Zylphrex marked this conversation as resolved.
Show resolved Hide resolved
if "thread.id" in transaction_data:
# The thread.id can be either a str/int. Make sure to always convert to a str.
transaction_ctx["thread_id"] = str(transaction_data["thread.id"])

# The hash and exclusive_time is being stored in the spans columns
# so there is no need to store it again in the context array.
transaction_ctx.pop("hash", None)
Expand All @@ -432,7 +439,7 @@ def _sanitize_contexts(
if app_ctx is not None:
app_ctx.pop("start_type", None)

# The profile_id and replay_id are promoted as columns, so no need to store them
# The profile_id, profiler_id and replay_id are promoted as columns, so no need to store them
# again in the context array
profile_ctx = sanitized_context.get("profile", {})
profile_ctx.pop("profile_id", None)
Expand Down
33 changes: 23 additions & 10 deletions tests/datasets/test_transaction_processor.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,28 @@ class TransactionEvent:
has_app_ctx: bool = True
profile_id: Optional[str] = None
profiler_id: Optional[str] = None
thread_id: Optional[int | str] = None
replay_id: Optional[str] = None
received: Optional[float] = None

def get_trace_context(self) -> Optional[Mapping[str, Any]]:
context = {
"sampled": True,
"trace_id": self.trace_id,
"op": self.op,
"type": "trace",
"span_id": self.span_id,
"status": self.status,
"hash": "a" * 16,
"exclusive_time": 1.2345,
}

if self.thread_id is not None:
data = context.setdefault("data", {})
data["thread.id"] = self.thread_id

return context

def get_app_context(self) -> Optional[Mapping[str, str]]:
if self.has_app_ctx:
return {"start_type": self.app_start_type}
Expand Down Expand Up @@ -161,16 +180,7 @@ def serialize(self) -> Tuple[int, str, Mapping[str, Any]]:
}
},
"contexts": {
"trace": {
"sampled": True,
"trace_id": self.trace_id,
"op": self.op,
"type": "trace",
"span_id": self.span_id,
"status": self.status,
"hash": "a" * 16,
"exclusive_time": 1.2345,
},
"trace": self.get_trace_context(),
"app": self.get_app_context(),
"experiments": {"test1": 1, "test2": 2},
"profile": self.get_profile_context(),
Expand Down Expand Up @@ -249,6 +259,7 @@ def build_result(self, meta: KafkaMessageMetadata) -> Mapping[str, Any]:
"trace.sampled",
"trace.op",
"trace.status",
"trace.thread_id",
"geo.country_code",
"geo.region",
"geo.city",
Expand All @@ -258,6 +269,7 @@ def build_result(self, meta: KafkaMessageMetadata) -> Mapping[str, Any]:
"True",
self.op,
self.status,
str(self.thread_id),
self.geo["country_code"],
self.geo["region"],
self.geo["city"],
Expand Down Expand Up @@ -345,6 +357,7 @@ def __get_transaction_event(self) -> TransactionEvent:
transaction_source="url",
profile_id="046852d24483455c8c44f0c8fbf496f9",
profiler_id="822301ff8bdb4daca920ddf2f993b1ff",
thread_id=123,
replay_id="d2731f8ed8934c6fa5253e450915aa12",
)

Expand Down
Loading