Skip to content
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 flush to HttpLogger #362

Closed
csp197 opened this issue May 25, 2019 · 6 comments · Fixed by #417
Closed

Add flush to HttpLogger #362

csp197 opened this issue May 25, 2019 · 6 comments · Fixed by #417

Comments

@csp197
Copy link

csp197 commented May 25, 2019

I want to use zipkin-js to trace requests handled by an AWS lambda function, written in NodeJS but the lambda seems to be dropping the request context prior to the async POST call to my Zipkin collector.

Currently, I'm using the ExplicitContext attribute from zipkin-js and the lambda function dies before the reporter can send the span.

I suspect the AWS Lambda drops the request context immediately after receiving the response, and since the reporter sends the POST request to the collector on a 1 sec cadence, the reporter is unable to send the span data.

A possible resolution may be to make the reporter synchronous (i.e: block the thread until the span is reported, and only then finish the request).

And another possible resolution may be to add a flush option, which would ideally asynchronously close the POST http request to the Zipkin collector, prior to finishing the request and the lambda shutting down.

Any advice is appreciated 👍

@jcchavezs
Copy link
Contributor

I believe this is a legit request. Do you feel like coming up with a PR?

@csp197
Copy link
Author

csp197 commented Jun 3, 2019

I'm happy to try, but I'm fairly new to javascript and its versatile syntax... :)

As I currently understand it, the processQueue function in HttpLogger.js on line 85 defines the fetch POST request to the Zipkin collector and the timer object on line 45 actually calls processQueue function at an interval (defined by the httpInterval variable).

My approach would be to add an optional integer variable named flush to the HttpLogger constructor, followed by an if/else statement in the setInterval function of the timer object. If the value of the flush variable is less than that of the httpInterval variable, then the setInterval will run with the flush value otherwise it will default to the httpInterval value.

I'm not sure how to make the make the instrumentation wait for the request to complete as the POST request is asynchronous..

@jcchavezs
Copy link
Contributor

jcchavezs commented Jul 19, 2019

What about passing -1 to the interval when you want to do the manual flushing? This way, on -1 the interval is never triggered and then you can flush it manually? Ping @adriancole

@codefromthecrypt
Copy link
Member

codefromthecrypt commented Jul 19, 2019 via email

@jcchavezs
Copy link
Contributor

Preparing a release today @csp197

@codefromthecrypt
Copy link
Member

https://github.com/openzipkin/zipkin-js/releases/tag/v0.18.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants