Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

Jaeger Implementation for Frontend #80

Open
oguzcankirmemis opened this issue Dec 18, 2018 · 5 comments
Open

Jaeger Implementation for Frontend #80

oguzcankirmemis opened this issue Dec 18, 2018 · 5 comments
Assignees
Labels
enhancement Improvement of existing functionality

Comments

@oguzcankirmemis
Copy link
Member

To implement Jaeger, we need to listen:

  • Every called middleware
  • Every called route (after middlewares)
  • Every call to other services (e.g Postgres, Redis, Backend...)

This way we can benchmark our code better and see exactly how much time every route consumes.

@oguzcankirmemis oguzcankirmemis added the enhancement Improvement of existing functionality label Dec 18, 2018
@oguzcankirmemis oguzcankirmemis self-assigned this Dec 18, 2018
@oguzcankirmemis
Copy link
Member Author

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.

@arkocal
Copy link
Member

arkocal commented Jan 15, 2019

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.

@oguzcankirmemis
Copy link
Member Author

With the new commits to PR #81:

  • Backend and redis calls are traced
  • Tags are now correct
  • A jaeger config is added to global frontend config to support both kubernetes and docker, also to be able to make changes easily

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.

@oguzcankirmemis
Copy link
Member Author

With the new commits to PR #81:

  • The bugs about the ordering of spans, and closing them are fixed
  • New tags added (e.g parent url tags, status code tags) are added

With this, this pr should be stable and we can merge it.

@oguzcankirmemis
Copy link
Member Author

Apparently for jaeger to work properly, we need to solve the context provider bug mentioned at #103.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Improvement of existing functionality
Projects
None yet
Development

No branches or pull requests

2 participants