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 X-Ray Service Map filter trace list query #203

Conversation

jamesrwhite
Copy link
Contributor

@jamesrwhite jamesrwhite commented Oct 5, 2023

This fixes #153

I have tested it on Grafana 8.4.11 and 10.1.2.

I tried to use the NodeGraphDataFrameFieldNames.subTitle enum directly to generate the query but was struggling to get that to build correctly. If there's a better way to achieve this please point me in that direction and I'll happily change it, I'm aware this approach is pretty hacky 😅

@jamesrwhite jamesrwhite requested a review from a team as a code owner October 5, 2023 15:43
@CLAassistant
Copy link

CLAassistant commented Oct 5, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@fridgepoet fridgepoet left a comment

Choose a reason for hiding this comment

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

Hey @jamesrwhite thanks so much for the contribution and your notes on the original issue!

To reproduce: I queried the Service Map, switched to Node Graph visualization, and chose a node and selected Traces > All.

I tested out the changes locally on:

  • v8.4.7 - OK
  • v8.5.1 - OK
  • v9.4.0 - OK
  • latest main v10.3.0-pre - OK

I was able to see the subtitle variable transform correctly.

src/DataSource.ts Outdated Show resolved Hide resolved
@fridgepoet
Copy link
Member

fridgepoet commented Oct 18, 2023

Hi again @jamesrwhite, I got some help from @idastambuk to write a test to describe what we're doing here. It's the last commit on another branch 3c0d131
Would you be willing to incorporate that?

@jamesrwhite
Copy link
Contributor Author

@fridgepoet Absolutely, thanks for that. I've just merged that branch into this one 🚀

Copy link
Member

@sarahzinger sarahzinger left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks so much for your contribution!

@fridgepoet fridgepoet merged commit b1b5f0c into grafana:main Oct 18, 2023
2 checks passed
@fridgepoet fridgepoet mentioned this pull request Oct 18, 2023
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.

Breaking change in grafana/data around 8.5 causing issues with links to trace querying in service graph
4 participants