Structured Logging

[zhiyan114]

Resolves #33

Feature:

• init type for logging
• Implement logger client function calls
• Implement beforeSendLog opt
• Implement scope attribute + attach to all the logs
• Implement Proper Transport and Flush System for logging
• Update LogServiceMessageOut.luau to support new logging system instead of using old, event-based message.
  • Should allow user to supply max logging level
• Update SentryClientRelay to support client-side Structured Logging
• Update docs to reflect the new feature?

[zhiyan114]

@devSparkle How does "ClientRelay" works?
Should I and patch the ServerEvent under "Integrations/SentryClientRelay" to support the log forwarding or would the InvokeFunction under "Transport" automatically handle that for me.

[devSparkle]

@zhiyan114 first of all, thank you for your work!

I've been studying the updated SDK documentation a bit. For us to remain in compliance with the documentation, this would have to be buffered in a layer between the logger and Transport, called the Telemetry Buffer, and then be sent over through :CaptureEnvelope.

The SentryClientRelay currently works only with the now deprecated /store endpoint and :CaptureEvent, not :CaptureEnvelope, so a new system would have to be developed. The question of whether or not the Telemetry Buffer should exist on the client and then batch process the sending to the server is also part of it. Performance differences would have to be tested.

There is also another question, about how we should handle separating client and server logs.

To put it simply — this will require a significant upgrade to the package.

[zhiyan114]

The question of whether or not the Telemetry Buffer should exist on the client and then batch process the sending to the server is also part of it. Performance differences would have to be tested.

On the client, I think it would entirely depend on the design of remoteEvent/Function. We could probably run a test to see if the client would perform better if the remote fire each event individually or in a batched manner. For the server, the only issue I can see is figuring out the best tradeoff between the size of the server buffer and number of endpoint request we make (maybe we could let the user to choose a value that fits their game or automatically scale based on some metrics like player count).

There is also another question, about how we should handle separating client and server logs.

I believe all custom-defined attributes are filterable on the dashboard, so we could simply use that to identify if the log originate from client or server. Something like origin=client/server

For now, you want me to finish other implementation and submit the PR (since I'd expect that it'll take a while for the re-write) or wait until the upgrade happens? For SentryClientRelay, I'll figure out a way to patch it without affecting the old implementation if we go with the former.

[devSparkle]

I'm perfectly happy with a patch release in the meantime!

[zhiyan114]

Everything looks good on my side.

Here's a sample test code I've used:
Server Script

local SDK = game.ReplicatedStorage.SentrySDK
require(SDK):Init({
	DSN= "DSN",
	enableLogs= true,
	beforeSendLog=function(e)
		print("EVENT",e)
		return e
	end,
	Integrations={}
})

warn("SERVER LOG AUTO CAPTURED!!")

Client Script

warn("CLIENT LOG AUTO CAPTURED!!")

I did observe a (studio bug?) that causes LogService.MessageOut to be fired on server script when client logs a message in-case you see duplicate log sentry (where one logger attribute shows "client" and the other shows "server")

[devSparkle]

This looks good! Did you account for logs generated by the SDK itself?

We don't want to cause a loop.

[zhiyan114]

So users aren't suppose to use print/warn inside beforeSendLog. I reviewed one of the sentry's SDK PR and saw that beforeSendLog are complete ignored if that's the case because of the infinite recursion issue you've mentioned. So instead, I just setfenv and make print/warn do nothing inside that function.

I tried to allow this behavior by dropping the log under MessageOut when the message are tagged (by padding the message with few null term. (\0)), but MessageOut happens to trim the null term. I could probably pad it with space char instead, but I rather not do anything if it has the chance to show up on the user's log.

I also do want to note that the fenv function is deprecated by roblox, but the alternative is just documenting this behavior and hoping users don't actually use print/warn inside that function.

[devSparkle]

So, the problem is that using setfenv disables a lot of Luau runtime optimizations.

Wouldn't it be more viable to prefix output from our own code with SentrySDK, and filter for it?

[zhiyan114]

It would, but the issue is that the loop would only occur if the user generates log content (calling print/warn) within beforeSendLog that matches the same level as the one that's being captured inside LogServiceMessageOut.

Using setfenv allows me to block the user from accidentally calling print/warn inside of it. For tagging, we could do that inside LogServiceMessageOut, it's not guarantee that's what the user will be printing out inside the beforeSend function.

I could probably still add that filtering just to drop any noisy log that's generated by the SDK itself, if any, but other than that, it's kind of a compromise we're taking here:

• Don't use setfenv; warn the user and hope that they don't accidently cause this issue.
• Use setfenv; block print/warn to prevent the accident from happening, but we loses optimization.