-
Notifications
You must be signed in to change notification settings - Fork 224
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
is apm.currentTransaction set inside a hapi server.events.on('response'
event?
#2353
Comments
From playing a little bit, the HTTP response has ended (and the APM instrumentation of core http has ended the transaction) before hapi sends the 'response' event. I think it would incorrect to bind "response" event handlers to the transaction: while it might help give the user access to How about using the 'onPreResponse' hapi request lifecycle extension point, e.g. https://gist.github.com/trentm/924b0ec426b6c81a3e76fab04f846655#file-trace-hapi-js-L30-L38 The HTTP response has not ended, the transaction context is current so // ...
// onPreResponse runs in the context of the APM transaction, so
// `apm.currentTraceIds` et al are available.
// For kibana, might need to make sure this runs after the
// `adoptToHapiOnPreResponseFormat` handler also added to 'onPreResponse'.
server.ext('onPreResponse', (request, h) => {
console.log('onPreResponse: traceIds=%j requestId=%s active=%s',
apm.currentTraceIds, request.info.id, request.active())
return h.continue
}) |
To answer the question posed by the issue title: No, the APM transaction for the request/response being reported in the hapi 'response' event is not the "apm.currentTransaction" in the response event handler. This means that one cannot get We could consider binding any 'response' event handler to this transaction run context. However there would be some caveats. By the time the 'response' hapi event is emitted, the HTTP response has ended and the APM transaction has ended. This means that only the readable parts of the Transaction API off of An answer to the above, as mentioned, is for the user to instead use Unless there is a specific user case made for needing the hapi server 'response' event handler bound to the transaction run context (i.e. using 'onPreResponse' instead is not feasible), I'm inclined to not add this functionality. I'm happy to reconsider (with a new issue or re-opening this one). @mshustov Please let me know if you disagree. |
I finally got to the elastic/kibana#114476 task.
Kibana Core controls access to Hapi AP, therefore it can enforce a custom
I don't say Btw We can work the problem around on the Kibana level. What if I store server!.ext('onPreResponse', (request, responseToolkit) => {
request.app.traceId = apmAgent.currentTraceIds['trace.id'],
});
server.events.on('response, (request) => {
console.log('traceId', request.app.traceId);
}); I wanted to test it with your snippet https://gist.github.com/trentm/924b0ec426b6c81a3e76fab04f846655 but when I run it with
|
I'm re-opening for discussion on improving our Hapi instrumentation and to get on planning radar. |
Yes, that sounds reasonable. It is nice that Hapi provides the explicit |
That block is a very good argument for binding the APM transaction to the 'response' event handler. |
In discussion with @mshustov working on elastic/kibana#102699 the
apm.currentTraceIds
inside a hapi server 'response' event are empty. Has the transaction already ended? Or is the run context just not bound to the 'response' server event. From a glance at the hapi instrumentation, I'm thinking the latter, but I need to play a bit to confirm.If that is the case can we and do we want the transaction run context to be bound to the server 'response' event? How about to other core server events?
The text was updated successfully, but these errors were encountered: