googleapis / google-cloud-dotnet

Google Cloud Client Libraries for .NET
https://cloud.google.com/dotnet/docs/reference/
Apache License 2.0
929 stars 365 forks source link

Add labels on a per-log basis to Stackdriver upload via Log4net #5325

Closed williecohen-google closed 3 years ago

williecohen-google commented 4 years ago

Is your feature request related to a problem? Please describe. I am using Log4net with the GoogleStackdriverAppender in a .NET framework project. I want to be able to add labels to our log entries for Cloud Monitoring. I want to be able to group logs easily using these labels (which is what their purpose seems to be). However, the GoogleStackdriverAppender only allows static custom labels to be added that apply to all logs flowing through the Appender.

Describe the solution you'd like Ideally there would be a way to attach labels to certain logs, and not just statically for all logs generated. I'm assuming this decision was made to try and "fit" Log4net nicely into the Stackdriver data model, but I would think there would be a way to accommodate per log labels.

Describe alternatives you've considered

amanda-tarafa commented 4 years ago

I'll take a look at whether we can do this and how, but just to set expectations, I won't get to it for at least a couple of weeks.

williecohen-google commented 3 years ago

Any updates? We are running into log query latency issues and using log labels would speed things up dramatically (because they are indexed).

jskeet commented 3 years ago

@williecohen-google: Sorry, not yet - we'll try to have a look next week.

amanda-tarafa commented 3 years ago

I'm working on this, will have an update early next week.

amanda-tarafa commented 3 years ago

I believe you can already achieve this using properties, custom labels and patterns for custom labels.

The only caveat is that if a property is not set, the pattern will return "(null)", so the label will always be set but with a "(null)" value in some instances. Either you filter out the ones with "(null)" values, or we could consider adding a StripNullValuedLabels property to the appender so that we can remove such labels if they existed.

Let me know if this sounds reasonable to you.

williecohen-google commented 3 years ago

Ok, I think this sounds reasonable. We have a default label, which we could set at GlobalContext to prevent null values, but it would probably be nice for other use cases to strip nulls out. I'll try this out and see if I run into any issues. Thanks!

williecohen-google commented 3 years ago

Ok I was able to successfully add log labels with this! I do think for a follow up:

  1. This should be documented somewhere (which maybe I missed).
  2. Adding a StripNullValuedLabels property would be useful for adding labels to select logs.

Thanks for looking into this!

amanda-tarafa commented 3 years ago

Thanks for confirming that this solves your issue.

  1. I'll make sure to add some docs/samples around this.
  2. We'll look int it.

I'll keep open until I've added the docs at least.

amanda-tarafa commented 3 years ago

I've added documentation on how to translate Log4Net properties into labels. You can find it here.

As for the StripNullValuedLabels property, I've moved this issue to our backlog in #5628 to wait for more user demand.

I'll be closing this issue now since we've done as much as we are planning to do for now, but do please add another comment if you think there's something else we can do.