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

update readme to be more specific about browser vs nodejs support #54 #312

Merged
merged 3 commits into from
Dec 11, 2018

Conversation

ewhauser
Copy link
Contributor

@ewhauser ewhauser commented Dec 8, 2018

This updates the README file to be more explicit about which parts of this project are available for node.js and the browser. This addresses #54 and is the first task in #311.

README.md Outdated Show resolved Hide resolved
@jcchavezs
Copy link
Contributor

jcchavezs commented Dec 9, 2018 via email

@ewhauser
Copy link
Contributor Author

ewhauser commented Dec 9, 2018

Not sure what you are asking for here. There already is an example in the project for the browser?

@jcchavezs
Copy link
Contributor

jcchavezs commented Dec 9, 2018 via email

@ewhauser
Copy link
Contributor Author

ewhauser commented Dec 9, 2018

Got it. That makes sense.

Once #314 is merged, all the unit tests will be running under both Firefox and Chrome which will provide integration tests for running under the browser.

As far as I can tell, there currently isn't a way to send local traces generated in the browser directly to a Zipkin server as the HTTP transport relies on node libraries. I'm planning on opening a separate issue to address that problem and think it would make sense to add an end to end test as part of that work.

@ewhauser
Copy link
Contributor Author

ewhauser commented Dec 9, 2018

Did a bit more digging and it seems that the first revision was inaccurate. It does appear that you can use the HttpLogger in the browser. The issue I was running into was when compiling it with TypeScript for an Angular application. The file correctly has the logic to use window.fetch instead of node-fetch at runtime, but the AOT compiler was trying to load the require statements.

I found a workaround for this and will update.

…d provide instructions for compiling with Typescript
@jcchavezs
Copy link
Contributor

@ewhauser shall we merge this?

@ewhauser
Copy link
Contributor Author

Yes, good to go

@webmutation
Copy link

@ewhauser so you have an Angular application working with zipkin-js ? i am currently working on this, but if this has been done I will stop to prevent duplication of efforts.

@jcchavezs jcchavezs merged commit aa95a28 into openzipkin:master Dec 11, 2018
@jcchavezs
Copy link
Contributor

jcchavezs commented Dec 11, 2018 via email

@ewhauser
Copy link
Contributor Author

@webmutation - Yes, I have a prototype that integrates Zipkin and Angular. I'm finishing up the README and tests but plan on making it available by the end of the week.

@webmutation
Copy link

Oh great news. Can you share the repo as is if its public.

@ewhauser
Copy link
Contributor Author

@webmutation I've made this available here: https://github.com/ewhauser/angular-tracing. Please keep in mind that it is an early prototype and there are probably dragons. Feel free to provide some feedback!

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

Successfully merging this pull request may close these issues.

4 participants