-
Notifications
You must be signed in to change notification settings - Fork 9
Extract base Javascript client from jaeger-client-node #1
Comments
Is the idea that the browser implementation would be talking (likely, via a reverse proxy of some sort) to jaeger-agent or to jaeger-collector? If the latter, you could use the collector's Zipkin JSON support, but it would have a slightly different architecture to the other jaeger clients. (PS: happy to try contribute an initial version here) |
It can talk to either agent or collector, doesn't really matter. Yes, through a reverse proxy, don't see many other options since it runs in the browser. I am not in favor of using Zipkin JSON in a brand new component. It should use Jaeger data model, which is richer than Zipkin's. |
FYI, the collector only has a jaeger thrift over http endpoint atm. If we want to send jaeger json, that has to be done first. |
It's still early days of us using and considering Jaeger. From my Zipkin-minded perspective, I'd have a preference to sending the spans directly to jaeger-collector. My understanding, however, is that skipping jaeger-agent would preclude dynamic sampling. I don't think I have enough vantage point to make that decision, so I'll defer. @yurishkuro I'd love if there was documentation on the Jaeger data model that makes it richer. It may change our internal strategy. |
Whether the JS client sends the data to the agent or the collector is not important for the client itself, it can always be configured to send to another URL. In the current implementation of Jaeger the agents are pass through anyway. You can see the data model in jaeger.thrift IDL. We don't have an official JSON schema yet, since the browser JS client would be the first use case where we needed it. @black-adder is correct that we'd need to define a JSON model and implement an endpoint accepting it. That same endpoint can be mounted to both the agent and the collector if needed. |
Regarding the richer data model, to quote from our CNCF proposal:
|
Thanks. I agree with you both that defining the JSON format would need to
happen soon. And I agree they can be mounted in either or both.
In the meantime, I can begin taking a look at pulling apart the Node client.
…On Oct 28, 2017 12:49 PM, "Yuri Shkuro" ***@***.***> wrote:
Regarding the richer data model, to quote from our CNCF proposal
<https://github.com/cncf/toc/blob/master/proposals/jaeger.adoc>:
Jaeger is built with OpenTracing standard in mind from the ground up,
including the backend, the data models, and the UI. Zipkin backend does not
support all of the features of OpenTracing, specifically structured k-v
span logs and multi-parent spans / DAGs (which are possible in OpenTracing
via span references).
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#1 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAQsZXmFXyUKlP4MnfKJnD77LaXhKIJpks5sw4VhgaJpZM4QAJOP>
.
|
Sorry to be that guy, but is there an ETA on this work? Should I wait or start looking for alternatives? |
@pavolloffay do you know when Kara is going to start the internship? |
The collector would make more sense to me today as the endpoint: we're already having to run an HA setup of collectors, but agents are currently much more disposable. |
Sorry for the delay @MarckK already started the internship and she is looking into this. Just cross-linking issue in node repo jaegertracing/jaeger-client-node#109 |
Any progress on this? |
Nobody is working on this at the moment afaik. |
Any progress on this? we also need it. |
@yurishkuro in Node.js client, you use flow to type check. In this project, can be used Typescript or PropTypes to do the same?? |
I am a fan of typescript, however, we don't want to maintain two separate JavaScript clients, here and for Node, so whatever is built for plain JS should be used in the Node client as a base. If this can be done with the base in TS, I'm for it. |
Project To-do's
@yurishkuro Is there a scope, outline, or README file for what this needs to do? |
we need this feature urgently. Can we contribute code ? Or do something else? |
Right now, I'm working in a fork of this repo, passing the base code of node's client to Typescript. Maybe we can create a shared fork and start to work in this to get a functional client ASAP. |
@sagrath23 is your objective a Node library in Typescript or a Web-compatible (not Node) library? If it's the latter, then why not do that development in this repository? |
@yurishkuro My goal is a web-compatible library. I didn´t start in this repo because I didn´t know that I can start here. |
You are welcome to start or migrate to this repo. We can give you maintainer status for it. I'm happy to provide code reviews. |
That's will be Awesome!!! I will migrate my job ASAP. |
I invited you as committer. I recommend starting with almost no code and iron out the build / CI setup first. Need to pick an NPM package name. |
Hey guys, I have been working on very similar thing for the past 5 months as a side project and I just saw this issue a week ago 💩 Anyway, what I've done is partially-implementing thrift binary protocol with plain javascript, so it can send spans directly to jaeger collector, and it runs on both node.js and browsers. However it uses modern-ish javascript APIs: For the past week, I've been working on refactoring the code, writing some documentation to initially share my work: https://github.com/dgurkaynak/stalk-opentracing-js/ Any feedback would be appreciated btw.
As a final note, I definitely agree on defining an official Jaeger-JSON format and jaeger-collector should accept this format via http requests. |
@dgurkaynak If you want, you can migrate your work to this repo. Your job looks very good!!! |
Thanks @sagrath23, but I think the purpose of this repo is a little different from mine. My work is just a workaround for sending spans directly to jaeger-collector from browsers. The right way of implementing this feature is already mentioned in jaegertracing/jaeger#723. And also stalk aims to offer a little more than that, like some opinionated typescript decorators which is not related with Jaeger at all. |
Hello, I'd love to have a jaeger client for the browser, and am happy to contribute, though it looks like some people have a good start on this already here. Is there a current working branch that has been pulled into this repo from the work that others have started? |
Hey all, in the mean time I had to get something working here, so I wrote a simple little JS/TS library that calls out to a server (written in Python) to actually execute the transactions: https://github.com/Quansight/jaeger-browser It is currently limited in scope to my current needs, but it works ok for those! import {Client} from 'jaeger-browser';
const client = new Client(new URL('http://localhost:8080/'))
async function doThings(originalExtractedScan: object): Promise<object> {
// Start a span based on some text-map encoded data
const span = await client.startSpanExtract({
name: 'new-span',
reference: originalExtractedScan,
relationship: 'child_of'
});
// create a new span under it
const newSpan = await client.startSpan({
name: 'other-span',
reference: span,
relationship: 'child_of'
})
// Finish this inner span
await client.finishSpan(newSpan);
// return the span we started text-map encoded
return await client.injectSpan(span);
} |
I guess the main focus of jaeger team is currently opentelemetry merge. Their javascript repo is pretty active and it supports both node & browsers: https://github.com/open-telemetry/opentelemetry-js |
Node.js implementation https://github.com/jaegertracing/jaeger-client-node is currently not compatible with running in the browser. The plan is to extract the core of the code from that implementation into this repository and only leave server-specific parts in the Node repo.
One of the proposed changes, necessary for the browser version, is to implement JSON over HTTP sender instead of Thrift over UDP.
The text was updated successfully, but these errors were encountered: