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

Support log name in StructuredLoggingHandler #842

Closed
jstafford5380 opened this issue Jan 24, 2024 · 1 comment · Fixed by #845
Closed

Support log name in StructuredLoggingHandler #842

jstafford5380 opened this issue Jan 24, 2024 · 1 comment · Fixed by #845
Assignees
Labels
api: logging Issues related to the googleapis/python-logging API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@jstafford5380
Copy link

jstafford5380 commented Jan 24, 2024

Is your feature request related to a problem? Please describe.

When configuring locally, i can set the logName by using client.setup_logging(name='my-api') and it works correctly. My logs show up as 'my-api' in the "select log name" dropdown in the log explorer. However, the moment I deploy it to GKE, it crashes stating the name argument is unexpected. Digging through the code, I noticed it is because, when deployed locally, it selects CloudLoggingHandler which accepts the name argument, but when it's deployed to GKE, it selects StructuredLoggingHandler which does not. This is frustrating for a couple reasons:

  1. It makes the code less portable because in order to avoid the error, the startup procedure needs to know where it's being deployed so it knows whether or not it should add the argument.
  2. If i don't set it, all of the logs show up in a log named 'python' which is confusing when you have multiple python components hosted in the same GCP project all logging to 'python'.
  3. I suppose I could just add a label to all logs with the application's name using logging.getLogger('my-api') but then that forces me to add extras to every log statement if I want to add labels to help further scope my logs.

Describe the solution you'd like
Support the log name in StructuredLoggingHandler. For comparison, our .NET solutions don't have this problem. It behaves the same way regardless of where it's deployed.

Describe alternatives you've considered
Point 3 of the problem statement. This is additionally problematic in a large enterprise where we're trying to build tools around log routing due to the inconsistency of how logs are partitioned. It's an inconsistent experience if nothing else.

Additional context

@product-auto-label product-auto-label bot added the api: logging Issues related to the googleapis/python-logging API. label Jan 24, 2024
@gkevinzheng gkevinzheng added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Jan 24, 2024
@gkevinzheng
Copy link
Contributor

gkevinzheng commented Jan 29, 2024

@jstafford5380 Unfortunately, StructuredLogHandler does not take in the name because in this logging mode, the underlying structured logging agent that propagates those logs to Cloud Logging hard codes the log name to be something like /projects/<something>/stdout. I can flag the handler not accepting name as a parameter (even if it does nothing) as a bug because it causes portability issues across different GCP environments.

As for point 3 of the problem statement, both StructuredLogHandler and CloudLoggingHandler support a labels parameter for attaching additional labels to every log statement.

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/python-logging API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants