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

send links to Jaeger reporter, if set #952

Merged

Conversation

hughsimpson
Copy link
Contributor

@hughsimpson hughsimpson commented Mar 5, 2021

Also:

  • Add a 'ChildOf' link type Removed for now
  • Bump com.dimafeng libs in elasticsearch module to help the failing test
  • Made the datadog test a little more forgiving (it appears that the type of exception thrown depends on the architecture or something)

Enables the following view in recent versions of Jaeger UI
Screenshot 2021-03-06 at 13 36 28

@hughsimpson hughsimpson closed this Mar 5, 2021
@hughsimpson hughsimpson reopened this Mar 5, 2021
@SimunKaracic
Copy link
Contributor

Nice PR, and thanks for the driveby fixes!
But this PR touches kamon-core, so we'll need to take a closer look at it!

Thanks for the patience :D

@hughsimpson
Copy link
Contributor Author

If it speeds up the process of review I can just remove the ChildOf case object, since it's the only thing being touched in core, and FollowsFrom is almost as good for my purposes

@SimunKaracic
Copy link
Contributor

SimunKaracic commented Mar 16, 2021

Yes please remove ChildOf, without that this should be a no brainer to merge :)

@hughsimpson
Copy link
Contributor Author

hughsimpson commented Mar 17, 2021

K reverted that. May make a separate pr though - I'd still like ChildOf 😄

@SimunKaracic
Copy link
Contributor

K reverted that. May make a separate pr though - I'd still like ChildOf

Please do, and feel free to write the motivation for it, so we know what ChildOf brings to the table that can't be modelled with FollowsFrom.

Thanks for the contribution :)

@SimunKaracic
Copy link
Contributor

Sorry for the conflicts! :D
If you have the time, fix them, if not, I'll deal with it tomorrow and merge :)

@hughsimpson
Copy link
Contributor Author

No problem, will do now

@hughsimpson
Copy link
Contributor Author

All done 👍

@SimunKaracic SimunKaracic merged commit 6b9dc53 into kamon-io:master Mar 19, 2021
@SimunKaracic
Copy link
Contributor

Great job on the whole PR, thanks for the contribution! 🎉

@hughsimpson hughsimpson deleted the sent_links_to_jaeger_reporter branch March 19, 2021 10:07
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.

2 participants