Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Adding opentracing headers for appservice requests #16226

Closed
MTRNord opened this issue Sep 2, 2023 · 2 comments · Fixed by #16227
Closed

Adding opentracing headers for appservice requests #16226

MTRNord opened this issue Sep 2, 2023 · 2 comments · Fixed by #16227
Labels
A-Application-Service Related to AS support O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.

Comments

@MTRNord
Copy link
Contributor

MTRNord commented Sep 2, 2023

Description:

Currently, synapse only sends the tracing span headers over federation (using the whitelist) and over replication. However, for appservice devs, it might also be nice to have the trace spans from synapse to follow the flow from the start instead of needing to manually find the right span.

For reference, I think it's probably just adding opentracing.inject_header_dict(headers, check_destination=False) somewhere in this area:

async def put_json(
or in
await self.put_json(
f"{service.url}{APP_SERVICE_PREFIX}/transactions/{urllib.parse.quote(str(txn_id))}",
json_body=body,
args=args,
headers={"Authorization": [f"Bearer {service.hs_token}"]},
)

However, I didn't work on the synapse codebase so far. So the pointers may not be accurate.

@clokep clokep added A-Application-Service Related to AS support S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Sep 2, 2023
@clokep
Copy link
Member

clokep commented Sep 2, 2023

Seems like a good idea, would be great to have a PR adding this.

@MTRNord
Copy link
Contributor Author

MTRNord commented Sep 2, 2023

Seems like a good idea, would be great to have a PR adding this.

I will see how my weekend plays out and do that. I am coming from actually having somewhat a need for this :) So I guess I can learn how to do a synapse PR :) This seems like a rather small change.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Application-Service Related to AS support O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Tolerable Minor significance, cosmetic issues, low or no impact to users. T-Enhancement New features, changes in functionality, improvements in performance, or user-facing enhancements.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants