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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add support for library instrumentation #713

Merged
merged 5 commits into from May 28, 2022

Conversation

losalex
Copy link
Contributor

@losalex losalex commented May 27, 2022

This feature provides an ability to log extra entry with diagnostics structure which contains logging library information. Such entry is logged only once when first entry is written by a process using logging library.

Fixes #714 馃

@losalex losalex requested review from a team as code owners May 27, 2022 04:09
@product-auto-label product-auto-label bot added size: m Pull request size is medium. api: logging Issues related to the googleapis/nodejs-logging-winston API. labels May 27, 2022
@losalex losalex added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 27, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 27, 2022
Copy link

@arbrown arbrown left a comment

Choose a reason for hiding this comment

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

Looks good. I just want to verify that you intend for the diagnostic entry to be after the first log and not before, since that's what the test looks like.

callback: () => void
) => {
assert.strictEqual(entry_, entry);
assert.deepEqual(entry_[0], entry);
Copy link

Choose a reason for hiding this comment

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

Should the test entry be the first, or should the diagnostic entry come first? Or does it matter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since I changed the logic in LogCommon.log() to write array of entries instead a single object, this logic just adopted to perform validations on array of entries

@losalex
Copy link
Contributor Author

losalex commented May 27, 2022

Looks good. I just want to verify that you intend for the diagnostic entry to be after the first log and not before, since that's what the test looks like.

Oh, I see - yep, seems I need later to make a fix in nodejs-logging to reverse an array. Good catch!

@losalex losalex merged commit 04f99b7 into main May 28, 2022
@losalex losalex deleted the losalex/feat-instrumentation branch May 28, 2022 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: logging Issues related to the googleapis/nodejs-logging-winston API. size: m Pull request size is medium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants