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

Include async call to close/flush LoggingWinston to use in Cloud Functions #598

Closed
dan-sproutward opened this issue Jun 2, 2021 · 5 comments
Assignees
Labels
api: logging Issues related to the googleapis/nodejs-logging-winston API. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. type: question Request for information or clarification. Not an issue.

Comments

@dan-sproutward
Copy link

Thanks for stopping by to let us know something could be better!

PLEASE READ: If you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.

Is your feature request related to a problem? Please describe.
Logging is dropped if a Cloud Function shuts down before LoggingWinston flushes.
Describe the solution you'd like
Asynchronous call that can be used to wait for LoggingWinston to flush before exiting function processing event(thread?).
Describe alternatives you've considered
Created a Promise that calls close() and end() and listens for finish before returning. This is not reliable and logging is still dropped.
Additional context
It is not clear to me how to flush LoggingWinston successfully. Calling close and end was a guess to force flush before exiting a function. This is not reliable.

@product-auto-label product-auto-label bot added the api: logging Issues related to the googleapis/nodejs-logging-winston API. label Jun 2, 2021
@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Jun 3, 2021
@freelerobot freelerobot added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. type: question Request for information or clarification. Not an issue. priority: p2 Moderately-important priority. Fix may not be included in next release. and removed triage me I really want to be triaged. labels Jun 4, 2021
@freelerobot freelerobot assigned simonz130 and unassigned freelerobot Jun 29, 2021
@yoshi-automation yoshi-automation added 🚨 This issue needs some love. and removed 🚨 This issue needs some love. labels Nov 3, 2021
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Nov 29, 2021
@SurferJeffAtGoogle
Copy link
Contributor

@dan-sproutward did chrismaree's pull request fix this issue?

@simonz130 simonz130 assigned losalex and unassigned simonz130 Jan 26, 2022
@simonz130 simonz130 added priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Jan 26, 2022
@dan-sproutward
Copy link
Author

@SurferJeffAtGoogle just saw your comment. my request was based on observations that we seemed to be dropping logging. I will need to revisit this and that won't happen for a little bit - I am in the weeds and had moved on from the codebase using this functionality. However, now that I know, I will get this back on my plate to see if it is making a difference. Thanks for the heads up.

@losalex
Copy link
Contributor

losalex commented Feb 7, 2022

Thanks @dan-sproutward for raising this issue.
I wonder if you tried the suggestion explained in awaiting-logs-to-be-written-in-winston and if yes, did it improved anything?

@losalex losalex added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Feb 10, 2022
@losalex
Copy link
Contributor

losalex commented Feb 28, 2022

@dan-sproutward , another way to resolve this issue is to expose the LogSync class as additional winston transport in this library. LogSync is recommended for usage in serverless environment since by default all logging output goes to stdout which is picked by the agent running on every node, so logging calls should not delay process significantly. Can you please let me know if this solution would be acceptable? Also, can you please share which parameters you use in LoggingOptions to create a LoggingWinston?

@minherz minherz removed the 🚨 This issue needs some love. label Mar 18, 2022
@yoshi-automation yoshi-automation added the 🚨 This issue needs some love. label Mar 18, 2022
@losalex
Copy link
Contributor

losalex commented Mar 22, 2022

@dan-sproutward , I believe that 676 provides a good alternative solution for this issue - please let me know if that helps. Closing this issue for now - feel free to reopen or comment if you have any issues.

@losalex losalex closed this as completed Mar 22, 2022
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. priority: p2 Moderately-important priority. Fix may not be included in next release. 🚨 This issue needs some love. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. type: question Request for information or clarification. Not an issue.
Projects
None yet
Development

No branches or pull requests

7 participants