-
Notifications
You must be signed in to change notification settings - Fork 35
Expose BacktraceMetrics.SessionId and BacktraceClient.AttributeProvider #232
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
Expose BacktraceMetrics.SessionId and BacktraceClient.AttributeProvider #232
Conversation
This is useful to allow SessionId and other attributes (e.g. 'guid') to be written to 'dying memory' on platforms that support it (e.g. Switch, PlayStation), which the Backtrace server is then able to parse on importing from platform crash reporting services, and set on the imported errors. It can then be correlated with other errors from the same session.
|
hey @hymerman ! |
|
Thanks! I've gone back and had a careful look over our usage of AttributeProvider and SessionId, and actually it looks like both can be replaced with BacktraceClient's indexer. IIRC the SessionId was needed since we were setting dying memory before enabling metrics on the BacktraceClient instance (which is the point at which the BacktraceMetrics added its session ID to the client's attributes), so having access to the session ID in advance was needed, but we don't do that any more - it's not much use having the session ID in dying memory anyway if we never got to the point where the Backtrace session metrics were sent anyway. It also felt cleaner using a member, rather than having to hardcode that we're requesting the "application.session" attribute. Perhaps exposing BacktraceMetrics.ApplicationSessionKey would be sensible? I can no longer remember why I thought AttributeProvider needed to be exposed - as you mention here, we can use the BacktraceClient indexer directly and it works fine! |
|
I think we can expose SessionId via metrics - but it would be better to keep attribute provider internal for now. |
|
Sure thing! What's the preferred way to amend a PR here? Should I push another commit with the changes and they'll be squashed here? Sorry, I've not used git or contributed many cls for a long time :D |
|
Yes - we can squash merge it - feel free to add more commits to each pull request |
|
hey @hymerman we can also modify your pull requests, if you would like to. We're about to prepare a new backtrace-unity release and I wonder what is the best way to move forward with including your changes in the next release. |
|
Hi Konrad,
Yes please, I think it's best if you modify them - we're also coming up to a release so my focus is 100% on that! Apologies for logging this and then going quiet!
…-------- Original Message --------
On 21/01/2025 18:03, Konrad Dysput wrote:
hey ***@***.***(https://github.com/hymerman) we can also modify your pull requests, if you would like to. We're about to prepare a new backtrace-unity release and I wonder what is the best way to move forward with including your changes in the next release.
—
Reply to this email directly, [view it on GitHub](#232 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AABF2BVESTUOUY2UZ3VUBPT2L2DWZAVCNFSM6AAAAABU54DCJ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDMMBVGQYDSNRZGU).
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
We will continue in #238 |
This is useful to allow SessionId and other attributes (e.g. 'guid') to be written to 'dying memory' on platforms that support it (e.g. Switch, PlayStation), which the Backtrace server is then able to parse on importing from platform crash reporting services, and set on the imported errors. It can then be correlated with other errors from the same session.