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

Fix Zipkin UI failing #644

Merged
merged 1 commit into from
Apr 19, 2020
Merged

Fix Zipkin UI failing #644

merged 1 commit into from
Apr 19, 2020

Conversation

shleshg
Copy link
Contributor

@shleshg shleshg commented Apr 17, 2020

When you are trying to send span to Zipkin without localEndpoint or with serviceName equals to empty string (as opentelemetry-go Zipkin exporter does), Zipkin UI is failing with error, while rendering traces with such error:

TypeError: "e.serviceSummaries is undefined"
    ua TracesTableRow.jsx:140
    React 6
    unstable_runWithPriority scheduler.production.min.js:19
    React 4
    Redux 6
    un traces-action.js:32
react-dom.production.min.js:209:194
    React 5
    unstable_runWithPriority scheduler.production.min.js:19
    React 4
    unstable_runWithPriority scheduler.production.min.js:19
    React 4
    Redux 6
    un traces-action.js:32

To fix this, I add mandatory serviceName parameter to NewExporter function.

Also add assert if serviceName == "" + test for it and fix the example.

@lizthegrey
Copy link
Member

You need to follow this step: https://identity.linuxfoundation.org/projects/cncf/individual-signup

@shleshg
Copy link
Contributor Author

shleshg commented Apr 17, 2020

I signed it

Kind: "SERVER",
Timestamp: time.Date(2020, time.March, 11, 19, 24, 0, 0, time.UTC),
Duration: time.Minute,
Shared: false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's surprising to me that this is what gofmt does, but the tests pass, so...

@jmacd jmacd merged commit cf7c4e5 into open-telemetry:master Apr 19, 2020
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.

5 participants