-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
feat(uptime): Allow uptime spans to be fetched from the organiation_trace
endpoint.
#96821
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
Conversation
92ec3fe
to
f23c3d9
Compare
public_alias="trace_id", | ||
internal_name="trace_id", | ||
public_alias="trace", | ||
internal_name="sentry.trace_id", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this break anything else or was this just a fix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a fix - we aren't relying on this id anywhere else in the ui, afaik.
Worst case it breaks something only in the sentry org where we have reading from EAP enabled. But I doubt it will even break anything there.
…trace` endpoint. This adds the `include_uptime` param to this endpoint. When this is passed, we'll fetch uptime results and make the main request the root span for all other items in the trace.
f23c3d9
to
252e661
Compare
row_dict["check_duration_us"].val_int if "check_duration_us" in row_dict else 0 | ||
) | ||
project_id = row_dict["sentry.project_id"].val_int | ||
project_slug = project_slugs[project_id] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Uptime Serialization Null Handling
The _serialize_columnar_uptime_item
function has two bugs:
http_status_code
andcheck_duration_us
are directly accessed via.val_int
onAttributeValue
objects without checkingis_null
. This can result in incorrect non-null values (e.g., 0) or errors when the field is actually null, bypassing theget_value
helper's null handling.- Accessing
project_slugs[project_id]
can raise aKeyError
if a project is deleted, asproject_id
from uptime results might not be present in theproject_slugs
map.
…meta` This adds `include_uptime` to the meta endpoint as well. When it's passed, we'll include the number of uptime spans in the meta as well Relies on #96821
def get_value(attr_name: str, attr_value: AttributeValue): | ||
if attr_value.is_null: | ||
return None | ||
resolved_column = columns_by_name[attr_name] | ||
if resolved_column.search_type == "integer": | ||
return attr_value.val_int | ||
elif resolved_column.search_type == "string": | ||
return attr_value.val_str | ||
elif resolved_column.search_type == "number": | ||
return attr_value.val_double | ||
elif resolved_column.search_type == "boolean": | ||
return attr_value.val_bool | ||
elif resolved_column.search_type == "byte": | ||
return attr_value.val_int | ||
raise ValueError("Unknown column type") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
feels like this should be a helper util somewhere else
kwargs.setdefault("response_body_size_bytes", None) | ||
return self.create_eap_uptime_result(**kwargs) | ||
|
||
def assert_expected_results(self, response_data, input_trace_items, expected_children_ids=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would this be useful in other tests?
I'll address feedback in a follow up pr so we can get this merged |
…meta` This adds `include_uptime` to the meta endpoint as well. When it's passed, we'll include the number of uptime spans in the meta as well Relies on #96821
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
This adds the
include_uptime
param to this endpoint. When this is passed, we'll fetch uptime results and make the main request the root span for all other items in the trace.