Skip to content
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

OS-7052 zoneadmd should create bunyan formatted logs #290

Merged
merged 7 commits into from Apr 22, 2020

Conversation

jasonbking
Copy link

No description provided.

@jlevon jlevon self-requested a review April 16, 2020 17:39
@jasonbking
Copy link
Author

I mentioned it in the ticket, but in case you're wondering.. since the output we need to log could contain embedded NULs, which would complicate using libbunyan (since it'd end up being escaped incorrectly because libbunyan would then escape the escaping) libbunyan does a lot of allocations for every message. Since we're already forming json, and just need to output a few extra properties to make the output Bunyan-compatible, I went that route. It also seems like since it's using fixed sized buffers allocated at startup, if a system gets into a bad spot, it might be more likely to be able to capture and log something.

usr/src/cmd/zoneadmd/log.c Outdated Show resolved Hide resolved
usr/src/cmd/zoneadmd/log.c Outdated Show resolved Hide resolved
usr/src/cmd/zoneadmd/log.c Outdated Show resolved Hide resolved
usr/src/cmd/zoneadmd/log.c Show resolved Hide resolved
usr/src/cmd/zoneadmd/log.c Outdated Show resolved Hide resolved

/* Buffer is too full to append "\\n". Force a flush. */
if (flushp != NULL && i >= dlen - 2) {
/* Buffer is too full to append "\\n" + NUL. Force a flush. */
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused by this existing code and the new version. What appended newline?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure either -- I think this was all to try to force a flush if the escaped version was too large to fit into the output buffer. Since custr_t already handles that, I think the setting of flushp and breaking out of the loop of the custr_append() call fails should handle that. I'll update accordingly.

usr/src/cmd/zoneadmd/log.c Outdated Show resolved Hide resolved
@jasonbking jasonbking requested a review from jlevon April 20, 2020 16:28
usr/src/cmd/zoneadmd/log.c Outdated Show resolved Hide resolved
jlevon
jlevon previously approved these changes Apr 21, 2020
@jasonbking jasonbking requested a review from jlevon April 21, 2020 23:55
@jasonbking jasonbking merged commit 2ef1197 into master Apr 22, 2020
@jasonbking jasonbking deleted the zone-bunyan-pr branch April 22, 2020 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants