[trytond] Tracing and transaction info

[n1ngu]

Add tracing to the Trytond integration and extract transaction information from RPC request objects.

[n1ngu]

Any insight on how to test this is very welcome

[n1ngu]

@pokoli sorry to bother you. Just pushed the tracing feature commit with a fair amount of metaprogramming black magic. I am still figuring out some typing annotation details but I'd enjoy your review on that part.

It is working just fine in some production deployments but I may have missed some thoughts for multi-tenancy deployments.

[pokoli]

@n1ngu starting from 6.8 series there is a new log feature, which is used by the tryton server to log events from the user. Maybe you will like to trace such events also in sentry. Do you think this is doable?

[n1ngu]

@n1ngu starting from 6.8 series there is a new log feature, which is used by the tryton server to log events from the user. Maybe you will like to trace such events also in sentry. Do you think this is doable?

Because that logging is implemented with a ModelStorage, any "ir.model.log":create call will get its own span with the present PR! Model.log could be instrumented just as create, copy, delete, etc, but there is very few code there. I think it would be more interesting to create spans for Transaction.commit or Transaction._store_log_records but that can be done out of the box with

sentry_sdk.init(
    traces_sample_rate=1/128,
    functions_to_trace=[
        {"qualified_name": "trytond.transaction.Transaction.commit"},
        {"qualified_name": "trytond.transaction.Transaction._store_log_records"},
    ],
)

because, unlike pool models, Transaction can be statically instrumented. Transaction.start is explicitly patched in this PR because I find the whole context manager it creates deserves a span but couldn't figure out how to do that with functions_to_trace (I feel I am missing a more obvious solution here)

I don't know if you meant to capture log events in sentry with capture_message to send events irrespective of unhandled exceptions and tracing sample rate? That'd be certainly interesting and easily doable! I'll give the idea a try in my staging deployments while this PR settles. I feel the major challenge will be configuring this not to wipe your sentry event ingestion quota :smiling_face_with_tear:

[n1ngu]

Thanks for your contribution, and thanks for your patience as we work through our backlog of community PRs.

Are you finished working on this PR? I noticed that you have added some TODO comments to your code. These should be addressed before we review the changes more thoroughly.

Hi @szokeasaurusrex , thanks for looking into it.

Those TODOs are more of a wish-list or statement that I believe it'd be worth the effort to work out a better implementation. Definitely not something that prevents this PR from working rather fine as is (we could remove those comments and move on). I recall I had addressed some of them in the implementation I am deploying in production, I should give it some love and update the PR.

Yet,

1. This integration caused malfunctions under certain conditions which are still under investigation. Because of this and the lack of tests (I'd need your help to address this) I'd flag this PR as experimental. Disabling tracing should keep the integration stable but I'd not feel confident merging this as is.
2. The PR deserves some love and a rebase but we won't be able to work on it in the foreseeable mid-term due to a drift in my organization. So, feel free to close the PR if keeping it open is too noisy for you (unless the Tryton community or the Sentry organization wants to take over the work)

Cheers!

[antonpirker]

Hey @n1ngu !

Thanks for again for your contribution. I will close this PR for now.

Hope you can pick it up until the waves of your organizations drift have settled. Sentry currently also does not have the resources to pick this PR up. All the best!