Skip to content

Conversation

@MadLittleMods
Copy link
Contributor

@MadLittleMods MadLittleMods commented Aug 13, 2025

Instrument encode_response with tracing in Sliding Sync requests

Just filling in the hole I see in the trace after current_sync_for_user. In this case, I know that it was probably just serializing all ~133k state events I requested for the Matrix HQ room.

Sliding sync request trace in Jaeger

The hole at the end after encode_json_response is already tracked by #17722 and we're adding tracing for it in #18804

Dev notes

POST https://matrix-client.matrix.org/_matrix/client/unstable/org.matrix.simplified_msc3575/sync
{
  "lists": {},
  "room_subscriptions": {
        "!OGEhHVWSdvArJzumhm:matrix.org": {
            "required_state": [ ["*","*"] ],
            "timeline_limit": 50
        }
    }
}

Pull Request Checklist

  • Pull request is based on the develop branch
  • Pull request includes a changelog file. The entry should:
    • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
    • Use markdown where necessary, mostly for code blocks.
    • End with either a period (.) or an exclamation mark (!).
    • Start with a capital letter.
    • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
  • Code style is correct (run the linters)

@MadLittleMods MadLittleMods changed the title Trace encode_response in Sliding Sync requests Instrument encode_response with tracing in Sliding Sync requests Aug 13, 2025
Comment on lines +736 to +739
set_tag(
SynapseTags.FUNC_ARG_PREFIX + "events.length",
str(len(events)),
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This matches the pattern we use elsewhere

set_tag(
SynapseTags.FUNC_ARG_PREFIX + "event_ids.length",
str(len(event_ids)),
)


return 200, response_content

@trace_with_opname("sliding_sync.encode_response")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This matches what we're doing for legacy /sync

@trace_with_opname("sync.encode_response")

@MadLittleMods MadLittleMods marked this pull request as ready for review August 13, 2025 23:46
@MadLittleMods MadLittleMods requested a review from a team as a code owner August 13, 2025 23:46
@MadLittleMods MadLittleMods merged commit 4cee8c7 into develop Aug 18, 2025
44 checks passed
@MadLittleMods MadLittleMods deleted the madlittlemods/sliding-sync-trace-encode branch August 18, 2025 14:29
@MadLittleMods
Copy link
Contributor Author

Thanks for the review @erikjohnston 🐖

@MadLittleMods MadLittleMods added the A-Tracing OpenTracing (maybe OpenTelemetry in the future) label Oct 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-Sync A-Tracing OpenTracing (maybe OpenTelemetry in the future)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants