Open oguzcankirmemis opened 5 years ago
After the first implementation, some fixes are done now to PR #81:
docker support for jaeger is added, this will potentially break the kubernetes implementation, but it is very fixable in my opinion (In kubernetes config, for the frontend service an alias for the 'localhost' should be created with the name 'jaeger'), @arkocal can you look and let me know if something like this is possible, if not we can look for alternatives.
'express-http-context' is changed with our own context provider ( I did not want to do that, because I think seperating the context of jaeger from the actual context of the requests itself is a good idea, but the 'express-http-context' module is too buggy with POST requests, see here). Now postgres calls gets correctly logged as a child span of the initial span of the request.
Remaining problems:
Currently the span for middleware and the request are not in child-parent relationship, and this causes that the middleware span is finished at the same time with the request span, which is not the real case, as the request processing comes after the middlewares. We could refactor the register function for requests so that it closes the middleware span before the request processing starts, but I am not sure whether this is the nicest solution, @arkocal what do you think about this?
Some middlewares are unnamed, probably because of the anonymous middleware declarations
Tags of the middlewares and routes are partially wrong (e.g tag is POST=undefined where it should be METHOD=POST or METHOD=USE for middleware), this is very easy to fix and I will resolve it in the next commit.
Backend and Redis calls are still not traced, but now I got the basics thanks to postgres instrumentation, I hope it won't be that hard to implement.
In the next commit these features are planned to be included:
Support for backend and redis calls
Correct tags
@arkocal let me know if you need anything else.
For checking whether the platform is running on docker or kubernetes, you can check the env variable KUBERNETES_PORT
, but please check the documentation to see whether it is a good idea. I will comment on the rest later.
With the new commits to PR #81:
In the next commit, I will be polishing the span handling (more and nicer tags (for example tagging requests that produced an error)). More or less, the jaeger implementation for frontend is done.
@arkocal let me know if you need something else.
With the new commits to PR #81:
With this, this pr should be stable and we can merge it.
Apparently for jaeger to work properly, we need to solve the context provider bug mentioned at #103.
To implement Jaeger, we need to listen:
This way we can benchmark our code better and see exactly how much time every route consumes.