-
Notifications
You must be signed in to change notification settings - Fork 123
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
Add tags from thread context to log #324
Comments
ScopedLoggingContext.newContext().withTags(Tags.of(USER_ID, userId)).run(() -> myRequest()); In your case, a "tag" looks like it will probably suffice since you say you only want to see the output in logs (not trigger any special behaviour in the backend because of it). See the full API in: By using gRPC contexts, you can also propagate the context between threads (e.g. for tasks being handled in worker pools). See the docs in https://grpc.github.io/grpc-java/javadoc/io/grpc/Context.html for more about how to propagate gRPC contexts. Note that you could (for extra debugging fun) conditionally add a One note of caution is that Flogger does not accept scoped data in the "implicit" context (e.g. code run without any context having been created). So if you "add tags" to the "current" context using the |
Thanks for the comment. I tried to use
Using pattern
Can you kindly comment what I'm doing wrong? Thanks. |
I'm not 100% sure how Log4J converts tags to the MDC contexts, so can't obviously say if what you're doing with formatting works. Can you try:
If you can get that working as expected, the switch to Also, if scoped contexts cannot be enabled there should be an error message in stderr explaining things. Can you check for that please? However the Log4J backend code is: And looking at it, I'm not sure I see where the metadata (including TAGS) is processed (other than be being formatted into the message string). I didn't contribute this code, so perhaps it needs adjusting? |
That works fine:
Prints:
How can we scope the tags? |
The code you had should have done it. Was there a message in stderr about not being able to find the grpc context implementation? |
Actually it occurs to me you probably need to set a system property: We are looking at making this automatic (since not everyone is in a position to set system properties) but perhaps that change hasn't happened yet. If you print the |
I have the following to set system properties:
Then, printing gives me the following:
What should I set the
No error message on stderr. |
Any pointers? I'll appreciate help as we'd like to roll this out, and set up observability on the tags. Thanks. |
If you examine the code I linked, the context data provider is created by this call to loadService:
So what you're using looks fine. I suspect the issue is that logging may have already occurred before the class you are adding your code to was statically initialized. What happens if you set the property on the command line before starting the JVM? The line |
It seems the feature you requested already exists, so I'll just close this issue now. |
I don't think this should be closed. I want understand what's missing from the docs and/or code so users don't run into these road blocks in future. |
Thanks for all the help and pointers. I was able to make this work. For future reference, the code and output is below. As you can see, there is a warning in the output now, but still, the log context is correctly being added: Pattern: Code: public static void main(String[] argv) {
System.setProperty("flogger.backend_factory",
"com.google.common.flogger.backend.log4j2.Log4j2BackendFactory#getInstance");
System.setProperty("flogger.logging_context",
"com.google.common.flogger.grpc.GrpcContextDataProvider#getInstance");
FluentLogger LOG = FluentLogger.forEnclosingClass();
System.out.println("******* CONFIG *******\n" + Platform.getConfigInfo() + "**********************\n");
ScopedLoggingContext
.getInstance()
.newContext()
.withTags(
Tags.of("USER_ID", "int64max"))
.run(() -> {
LOG.atInfo().log("Processing user request.");
});
} Output:
|
If you look at the code, the warning means that it didn't find a getInstance() method, so used the constructor instead. I think you can just get rid of the #getInstance suffix for the GrpcContextDataProvider. This "plugin" mechanism has gone through a few iterations and tries to be flexible about whether there's a static factory method or not. However, what you are doing currently will fail as soon as a class referenced by your main class uses logging in its static initialization. It's entirely possible for a lot of code to be run before your "main" method is executed. These values really need to be set on the JVM via flags, not set in code. |
I had actually initially set
Yes, I understand this and thanks for pointing out. I had just left it here as an example. In practice, we set it in a static block in the class to which main belongs. So all is fine. |
""In practice, we set it in a static block in the class to which main belongs. So all is fine."" Interesting. If the static block is the first thing in the class (before static fields) then I think that could work (it's not an approach that had occurred to me before). I don't recall if the JVM loads certain dependencies for the "main" class before it begins static execution, or whether dependency loading is only done as a result of static initialization (which should happen in top-to-bottom order). Note that, of course, you'll need to be sure that you don't accidentally call any other libraries which might do (flogger) logging before setting the properties, but that's not too hard. Please let me know what sort of documentation changes you think would have helped you not need to ask this question (I am well aware that Flogger's docs need some care and improvement. If you wanted to send this as a PR, just to make clear what you mean, I'd be happy to incorporate it. |
I think something as simple as the following will go a long way to direct users to the right path. Logging ContextYou can use tags and metadata to automatically add information to logged lines. This is akin to Fish Tagging or Thread Context in Log4j (not sure you want to refer to Log4j in flogger documentation). To use tags, you need to provide a logging context first. This can be done by setting the Best practice: you can use scoped context to automatically unset the tags. For example: try (var scope = ScopedLoggingContext
.getInstance()
.newContext()
.withTags(
Tags.of("USER_ID", "int64max"))
.install() {
logger.atInfo().log("Procesing request.");
} |
Thanks, I'll see about making improvements to the docs. It can be difficult for me to realize what's clear or not in the open-source docs because (a) I designed all the APIs, so don't need the docs and (b) we have additional docs for Flogger inside Google which are what I primarily use/maintain. So thanks for helping out and giving me the perspective of someone from the "outside" :) |
I want to achieve something similar to fish tagging in log4j (https://logging.apache.org/log4j/2.x/manual/thread-context.html)
Basically, we have a multi-component system receiving requests from users. At entry point, I want to add the user's ID to the thread context, and then each log line output by any sub-component should automatically include the user ID in the log. Asking each component to add the ID explicitly is error prone.
From what I understand looking at Flogger, this is what "tags" and "metadata" are supposed to be for. Though I don't see how we can use these. I don't want to use
LOG.atInfo().with(USER_ID, userId).log()
everywhere.I've seen #309 and #145 but they don't seem to have a conclusion, and seem overly complicated for this use case.
What are my options? Is this not supported by flogger? If so, which classes should I extend to achieve my goal?
Thanks.
The text was updated successfully, but these errors were encountered: