logtail / logtail-js

Better Stack JavaScript client
https://betterstack.com/logs
ISC License
54 stars 13 forks source link

Logtail complains about an issue instead of fixing it #125

Closed ivan-kleshnin closed 2 months ago

ivan-kleshnin commented 2 months ago

LogtailJS does not omit/mask the object fields violating the contextObjectMaxDepth setting.

Instead, it complains and asks ME to prepare data accordingly:

[Logtail] Max depth of 2 reached when serializing logs. Please do not use excessive object depth in your logs.

Sorry guys, but it doesn't resemble an adequate handling 😠 If we have to write recursions, to obey library configs for its own input... the library fails to do its job.

There's also contextObjectMaxDepthWarn but it does not fix the root of the issue. I'm convinced the logger has to accept any valid JSON and replace "too deeply nested" fields with some placeholder like ***. You simply can't ask users to carefully measure, for each logging line, whether the local data (sometimes dynamic and unpredictable) can potentially violate some library setting. The depth setting is basically unusable a.t.m

curusarn commented 2 months ago

Hi @ivan-kleshnin!

Thank you for raising this! This doesn't seem like correct behavior.

I've passed this on to the team to take a look. We'll keep you updated here in GitHub.

Thanks again for the report!

PetrHeinz commented 2 months ago

Hello @ivan-kleshnin and thank for reaching out about this 🙌

If I understand you correctly, the library does exactly what you're suggesting - it replaces the too deep data with a placeholder (with e.g. <omitted context beyond configured max depth: 2>).

It also triggers a warning to warn you about possibly omitting important data, so you can reconfigure the max depth (via contextObjectMaxDepth option, which is 50 by default) or tweak your data. You can also turn the warning off (via contextObjectMaxDepthWarn option):

I've quickly tested it, and seems to behave as expected:

const logger = new Logtail(sourceToken, { sendLogsToConsoleOutput: true, contextObjectMaxDepth: 10 });

logger.info("Nested info", { a: { b: { c: { d: { e: { f: { g: { h: { i: { j: { k: { l: { m: { o: { p: { q: 123 } } } } } } } } } } } } } } } });
image

Could you please provide more info on how it behaves in your case and what behavior do you expect? 🙏

ivan-kleshnin commented 2 months ago

Thanks for responding. Yes, you're right – there's no issue and it behaves as expected.

Another subsystem affected the log and, from the wording of the message, I mistakenly derived the conclusion. Should have checked it in isolation 🤦‍♂️ Closing.

PetrHeinz commented 2 months ago

Thanks for the quick reaction, @ivan-kleshnin

No worries, happy to double-check everything works as intended on our end 🚀 Glad I could help!