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

InfluxDB: Fix table parsing with backend mode #76899

Merged
merged 7 commits into from
Oct 23, 2023

Conversation

itsmylife
Copy link
Contributor

@itsmylife itsmylife commented Oct 20, 2023

What is this feature?

This PR is addressing a couple of issues altogether.

  • We were using custom table parsing logic for backend mode. That's not needed anymore
  • Also we were adding rowName to the field names. For instance when you query something select qqq, eee from somewhere we return the names as somewhere.qqq and somewhere.eee. This is not compatible with what we have with the frontend mode.
  • Panels with influxdb frontend mode if have transformations (i.e. join by field) is failing since backend is sending field names with rows
  • Table parsing logic relies on query model but with raw queries we don't have it. So it is not working.

Who is this feature for?

InfluxDB influxql users with backend mode enabled.

Which issue(s) does this PR fix?:

Fixes #76897

Special notes for your reviewer:

How to test

  • Set feature toggle influxdbBackendMigration=false
  • Create a panel that uses InfluxDB InfluxQL raw code editor in frontend mode
  • Use query SELECT usage_idle, usage_iowait FROM "cpu" WHERE $timeFilter
  • Select "Table" format
  • Apply transformation "Join by Field"
  • Apply field override to usage_idle and usage_iowait
  • Run the query and observe you got Time, usage_idle, usage_iowait columns back
  • enable feature toggle influxdbBackendMigration=true
  • Refresh and see the table is not same or potentially broken
  • Switch to this branch and run the same query on same panel
  • Observe you have the same result and transformation, field overrides are working

Release notice breaking change

For the existing backend mode users who have table visualization might see some inconsistencies on their panels. We have updated the table column naming. This will potentially affect field transformations and/or field overrides. To resolve this either:

  • Update transformation
  • Update field override

@itsmylife itsmylife marked this pull request as ready for review October 20, 2023 19:28
@itsmylife itsmylife requested review from a team and grafanabot as code owners October 20, 2023 19:28
Comment on lines +258 to +261
if resultFormat != "table" {
frameName = append(frameName, row.Name...)
frameName = append(frameName, '.')
}
Copy link
Contributor Author

@itsmylife itsmylife Oct 20, 2023

Choose a reason for hiding this comment

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

We remove those in table responses for frontend mode compatibility

@itsmylife itsmylife requested a review from gabor October 20, 2023 19:39
@itsmylife itsmylife added add to changelog breaking change Relevant for changelog generation and removed no-changelog Skip including change in changelog/release notes labels Oct 20, 2023
Copy link
Contributor

@bohandley bohandley left a comment

Choose a reason for hiding this comment

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

LGTM!

@itsmylife itsmylife merged commit 63abe25 into main Oct 23, 2023
25 checks passed
@itsmylife itsmylife deleted the ismail/influxdb-table-parsing-fix branch October 23, 2023 17:21
grafana-delivery-bot bot pushed a commit that referenced this pull request Oct 23, 2023
* Add resultFormat to query model

* don't add row name if the result format is table

* No need special formatting since we use unified dataframes

* betterer

* specify visualization type in response

* Unit tests

* fix unit tests

(cherry picked from commit 63abe25)
itsmylife added a commit that referenced this pull request Oct 23, 2023
InfluxDB: Fix table parsing with backend mode (#76899)

* Add resultFormat to query model

* don't add row name if the result format is table

* No need special formatting since we use unified dataframes

* betterer

* specify visualization type in response

* Unit tests

* fix unit tests

(cherry picked from commit 63abe25)

Co-authored-by: ismail simsek <ismailsimsek09@gmail.com>
@aangelisc aangelisc removed this from the 10.3.x milestone Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

InfluxDB: Table rendering with transformation is broken when switching to backend mode
3 participants