-
Notifications
You must be signed in to change notification settings - Fork 96
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 for labels in log entry #152
Support for labels in log entry #152
Conversation
)" This reverts commit c71e35f.
@arthurdarcet It would help if you outlined your use case in the PR description above (just edit) and explained what parts of it aren't addressed by the existing |
From my understanding, below is how [How label_map is used] In configuration
User's json:
Resulting json
Resulting labels
[How ad-hoc labels are used] User's json:
Resulting json
Resulting labels
|
@igorpeshansky We are running a Kubernetes cluster managed by Google Container Engine. So GKE manages for us the fluentd daemon, and tweaking the configuration is really tricky (to say the least) Beside, this fluentd plugin might be collecting logs from a very high number of apps ; using a configuration option means that you have to add configuration entries for each app-specific label, something that should instead be managed by the app itself. It seems much cleaner to have the app output "logging.googleapis.com/labels: {my_label: val}" instead so that this daemon picks it up on its own |
Hi @arthurdarcet, I've pinged our product owner to let her know the feedback. As for the possibility to customize Fluentd config in Kubernetes, I believe there is some on-going work (not sure if it's released yet). @crassirostris, would you mind to chime in? The question is whether customers can customize Fluentd config in Kubernetes. If yes, which version do they need to upgrade to? If not, any ETA? |
It's possible to customize the fluentd logging agent, here's the documentation for that: https://kubernetes.io/docs/tasks/debug-application-cluster/logging-stackdriver#configuring-stackdriver-logging-agents. I think it's not that complicated after all: you should turn off the default logging solution and deploy the same DaemonSet + ConfigMap you had before, with your changes in the ConfigMap. I'm ready to assist with that and improve the docs if they're less helpful that desired However, unfortunately, customizing the configuration means it's not possible for GKE to support and predictably update it and you have to do it yourself |
@crassirostris The page you linked warns:
Which is what I mentioned earlier: for a Kubernetes cluster deployed by Container Engine (GKE), the fluentd DaemonSet is managed by Google. Any attempt to modify it (for instance to update the config with a new label map), will be reverted to the default after a few seconds. The only way we found that let us update the fluentd config was to add a I think there are major issues with this:
|
Sorry, that's my bad, documentation is stale,
Agree, but the problem is that when the custom configuration is broken, there's no clear understanding what "fix" exactly means and what behavior is expected in the end
That actually sounds reasonable @igorpeshansky The case is: running the same configuration with different application, each of them setting the labels, operation and so on independently. That'll also help the logging client libraries to adapt the stdout/stderr approach without loosing the ability to set those fields: instead of sending the logging entry directly to the API, they can just write specially formatted json to the stdio |
Another use case: labels can have the information about Docker splitting the log line (details in moby/moby#34855) without changing the payload type |
I came here searching for this exact same functionality. I too would prefer not to have to change the default logging agent on our GKE cluster, and found this page to help us understand how to set https://cloud.google.com/logging/docs/agent/configuration#special_fields_in_structured_payloads Unfortunately, it didn't mention anything about labels, which is why I ended up in this repository, and found out this is currently not supported. It would be great if as a side-note: at our company, we wrote a small Go library for easier Stackdriver logging from our GKE cluster; zapdriver, it currently supports setting the |
@qingling128 @crassirostris what actually prevents this PR from being reviewed and merged? |
This PR did not pass PM review the last time since the functionality is considered duplicate from https://cloud.google.com/logging/docs/agent/configuration#label-setup. The instruction to customize gke is available at https://cloud.google.com/monitoring/kubernetes-engine/customizing. If there is a use case that is not covered, we could revive the feature request. |
@qingling128 Ok, thanks for looking this up. I guess I would say the part that isn't covered is this:
but I won't argue beyond this here: I understand the logic, and that is definitely a product decision. We have moved away from Stackdriver Logging anyway, partly because of the price point for our log volume, and partly because configuring it to display what we needed wasn't easily doable. I guess the "customizing k8s" documentation now helps, but I don't think paying the high price point for stackdriver logging is worth it if you still have to manage the fluentd/… config manually. |
I'd like to add my voice that it's strange that this isn't supported out of the box by GKE. Sure, we can start fiddling with the logging agent configuration, but the reason we pay for GKE combined with Stackdriver is that it's supposed to "just work". I don't consider this duplicate functionality, as one requires manual configuration on GKE, the other (this PR) doesn't. See also: blendle/zapdriver#4 |
@arthurdarcet & @JeanMertz - Thanks for your inputs. These are valuable to us. I've added this feature request to our next product sync. I've summarize the issue as below.
Feel free to add or emphasize any other use cases if I missed any. |
Due to the holiday schedule, our next product sync would be on Jan 8. |
Updates on this feature request: It has been approved by product. The related development and documentation work will be prioritized. |
This is being implemented in #292. |
Reverts #151