-
Notifications
You must be signed in to change notification settings - Fork 4
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
How to add custom JSON properties #26
Comments
Unfortunately there is not currently a way to do what you are looking for, nor do I think we could easily add a way into AL. The problem lies in that a call to Furthermore, I think the solution you are proposing has a severe limitation... Because you are setting the gcpTrace as a variable you are wildly opening yourself up to a race condition, especially if you put your express app under pressure. gcpTrace would easily get changed out from under you when the various requests are processed "simultaneously" especially if you are using async/await at all in your request handlers. As I see it, the problem is two-fold... How do you get the That said, I think your best bet, knowing your specific code base as I do, would be for you to just send the trace context value as an argument in the log message args, and then in DataDog, alias that into the field you need it to be. DD is good at things like that.
But I think you want to extract the trace context value out in the request handler, not in some middleware, if you want to avoid the race condition stuff.
You could also consider creating a |
Actually was just looking at the code and you might be able to do this...
So basically you are just calling |
You've hit the nail on the head with the race condition. Because the logger is global, and the request is not, the global logger will never be able to log the traceId. Even with a wrapper, you still have the problem. Your second comment there is also correct, that does work. But of course making sure the developer 1) has access to the token everywhere, and 2) changing the way it's called. I think the best solution i've come up with, is to use The problem remains though, if you have any classes, they somehow need access to the request, which you really don't want common classes to have know about a request. Not great solutions as far as I can tell. |
Well you are essentially just moving the wrapper logic I suggest to the request context, but thats fine. I’d still avoid doing any work in the middleware and instead just try to inline it. some frameworks “thread” the middleware stuff so you could be right back in the race condition boat. As for the nested/dependent class/code stuff, yea, that’s gonna be a problem. One idea might be to have a call that essentially creates a log transaction, and then you can hand the transaction around, but you, the end user of AL would still be responsible for passing the transaction everywhere. Not ideal. I’ll think on it some more but I’m not optimistic. |
I have an idea for a partial solution on my end... you will still need to solve the problem on you end though. From AwesomeLog's side... I can create a new class called LogEntryOverride which takes an object during construction. You can then pass an instance of LogEntryOverride into the This thus lets you mutate a LogEntry on a one off approach, as opposed to the hooks approach which is more global. Example...
|
I have published v4.4.0 with the above LogEntryOverride included. Let me know if that helps. Please, please, please test this version before putting into production. v4.3.0 and v4.4.0 have changes that could potentially cause issues for your system. |
@arei I was able to figure all this out. It was complicated. But basically, I needed a tracing tool first. I used OpenTelemetry for that. Then, I was able to, using the beforeWriteHook, get the active trace & span, and inject that into the log. B/c the tracer is global, it doesn't have to be in the http context. export const instrumentLogger = (logEntry) => {
const span = trace.getSpan(context.active());
if (span) {
const spanContext = trace.getSpanContext(context.active());
logEntry.spanId = spanContext.spanId;
logEntry.traceId = spanContext.traceId;
}
}; The only thing we have left to deal with, is that the Whitebox ConigService cannot be overridden to take the beforeWrite hook, but that's easily modified. Just wanted to update you on the saga. Thanks for helping out with it as well! |
As an aside, if you wanted to, the other big logging libraries have built-ins for openTelemetry. npm install @opentelemetry/instrumentation-bunyan These look too complicated for me to make one on my own, but it's something you could offer one day if you wanted to. |
Hello dear sir,
We would like to use AwesomeLog to log the GCP traceId of cloud-run. From Googles perspective, they want you to add a field to the JSON of the log.
https://cloud.google.com/run/docs/logging#writing_structured_logs
The only way I can tell to add something to the logEntry JSON is to use a beforeWrite hook. The problem with that, is that that is configuration level. I don't have the traceId until i'm in the middleware of Express
So my present way of doing this, which is very ugly, is to delay starting AwesomeLog until AFTER the middleware has run, so that when the hook is defined, I have the value.
Is there any cleaner way to add a custom field to the JSON, something on
Log
that can be adjusted in that middleware? Something likeLog.customFields['logging.googleapis.com/trace'] = gcpTrace
instead of having to define this in the configuration.Thanks!
The text was updated successfully, but these errors were encountered: