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

allow sending extras when setting up client #149

Open
daniel-sanche opened this issue Jan 9, 2021 · 7 comments
Open

allow sending extras when setting up client #149

daniel-sanche opened this issue Jan 9, 2021 · 7 comments
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@daniel-sanche
Copy link
Contributor

We recently added a feature to allow log commands to send in extras, allowing users to customize certain fields (#129)

@nicoleczhu suggested expanding this by allowing users to set default extras when first setting up the logging client

@product-auto-label product-auto-label bot added the api: logging Issues related to the googleapis/python-logging API. label Jan 9, 2021
@daniel-sanche daniel-sanche added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed api: logging Issues related to the googleapis/python-logging API. labels Jan 9, 2021
@daniel-sanche daniel-sanche self-assigned this Jan 9, 2021
@daniel-sanche daniel-sanche modified the milestones: v2.4.0, v.2.3.0, v2.5.0 Jan 27, 2021
@oittaa
Copy link

oittaa commented Feb 20, 2021

It would be great if trace (X-Cloud-Trace-Context header) was included automatically. For example Cloud Run logging is super confusing. The documentation just points all around without leading to any clear solutions and Google-searches direct to some random GitHub repos with people having logging issues.

So please consider adding tracing even in the simplest Cloud Run apps that are basically

import logging
logging.info("message")

snarfed added a commit to snarfed/webutil that referenced this issue Mar 6, 2021
…e id

makes log lines attach to their HTTP request and show grouped in cloud console log browser. google-cloud-logging used to do this automatically for webapp2, based on the X-Cloud-Trace-Context header, but it dropped webapp2 as of google-cloud-logging 2.0.

background:
googleapis/python-logging#110 (comment)
googleapis/python-logging#149 (comment)

also note that AppEngineHandler is evidently deprecated, so I may need to port that whole class into webutil eventually. :(
googleapis/python-logging#202
@daniel-sanche daniel-sanche modified the milestones: v2 Update 2, v2 Update 3 May 4, 2021
@daniel-sanche daniel-sanche modified the milestones: v3.0.0 Breaking, 4.0.0 Breaking Nov 16, 2021
@arbrown arbrown added the priority: p3 Desirable enhancement or fix. May not be included in next release. label Jul 12, 2022
@spiqueras
Copy link

It would be great if trace (X-Cloud-Trace-Context header) was included automatically. For example Cloud Run logging is super confusing. The documentation just points all around without leading to any clear solutions and Google-searches direct to some random GitHub repos with people having logging issues.

So please consider adding tracing even in the simplest Cloud Run apps that are basically

import logging
logging.info("message")

Please. It is mind-blowing how badly these libraries integrate with Logging Explorer. This two things:

have been requested multiple times, and were already a thing in all Gen1 App Engine environments, yet people have to resort to third party solutions (such as https://github.com/salrashid123/flask-gcp-log-groups) to make the logs bearable. I can't think of any use case where you wouldn't want those things to just happen.

@daniel-sanche
Copy link
Contributor Author

Hey @spiqueras ,

It's surprising to me that you're still seeing these issues. The library does attempt to parse X-Cloud-Trace-Context and w3 traceparent headers for trace correlation. The Log grouping backend behaviour has changed since App Engine Gen1, but it should now group based on http_request metadata. The library should attempt to populate these http fields automatically for Flask and django, which should result in log grouping (although maybe not exactly like the old App Engine Gen 1 method).

It's possible the issues you're seeing are due to a bug in the library, or it could be a configuration error, or it could just be an issue with the environment you are running the code in. If you can give me more information of how you're running your code, example logs, and/or sample code that reproduces what you're seeing, I can try to take a look.

If you have an example of a log output by the library that's ungrouped vs a similar one that is grouped how you expect, that would be especially helpful in tracking down the root cause

Thanks,
Daniel

@daniel-sanche daniel-sanche removed this from the 4.0.0 Breaking milestone Dec 13, 2022
@pnico
Copy link

pnico commented May 8, 2023

For anyone using this library who is only using it because they want to get log correlation like they had before with appengine standard, and who doesn't need to actually call functions in the cloud logging API, you don't need to use this library at all. You might have to change the way you are looking at logs, or in the way you are analyzing their output if you are forwarding them to BigQuery or elsewhere using log sinks and depend on protoPayload.line.logMessage etc - that seems to be just the way it is.
Please check here and here - this was all we needed.
The example in the first link is for cloud run but also works for appengine. It just shows you how to do this using print() - which I guess you could do, but really I'm not sure why one would want to. You can create a subclass of LogFormatter that returns the same json-dumped string as described there from its format method, set it as the formatter of the root logger, and you'll have correlated logs without using this library at all. You can have logging.info("Hi, this is a plaintext string") and have all app logs such as this properly grouped per request, including the usual metadata you'd expect such as severity (see the second link for how to do that), without importing this package.

I'm not sure why they don't recommend this approach by default for appengine customers in this situation, and maybe there's a good reason I'm not aware of, so take this comment with a grain of salt. I'm sharing this here because it would have saved me many hours of wasted time if I had seen a comment like this.

@daniel-sanche
Copy link
Contributor Author

Hey pnico,

Yes, you are correct that App Engine and certain other GCP environments will automatically pick up formatted stdout logs. This library provides the StructuredLogHandler to take advantage of this, which essentially does what you describe, by sending logs through a custom Formatter and Handler to export properly formatted logs over stdout, to avoid the overhead of network calls.

This library will attempt to determine which environment your code is running on, and pick the best log export method based on the environment. But you are correct that if you only plan on running your code on App Engine, you can avoid this library and just write structured json logs directly (using our StructuredLogHandler, or custom code doing something similar)

@pnico
Copy link

pnico commented May 8, 2023

Yeah, I looked at some of the handler classes there but importing any of those seems to bring in the rest of the library

python3
Python 3.11.2 (v3.11.2:878ead1ac1, Feb  7 2023, 10:02:41) [Clang 13.0.0 (clang-1300.0.29.30)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import psutil
>>> psutil.Process().memory_info().rss / (1024 * 1024)
12.63671875
>>> from google.cloud.logging_v2.handlers import StructuredLogHandler
>>> psutil.Process().memory_info().rss / (1024 * 1024)
53.328125
>>> 

Maybe those handlers add some details that could be useful, but we can get everything we need with something much simpler. GCP is actually adding resource labels module_id, project_id, version_id automatically, even.

@daniel-sanche daniel-sanche removed their assignment Oct 20, 2023
@daniel-sanche
Copy link
Contributor Author

This could be addressed with an adapter: #750

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

5 participants