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

Let user to specify "format as" in query builder #300

Closed
jkaflik opened this issue Feb 13, 2023 · 8 comments · Fixed by #329
Closed

Let user to specify "format as" in query builder #300

jkaflik opened this issue Feb 13, 2023 · 8 comments · Fixed by #329
Assignees

Comments

@jkaflik
Copy link
Collaborator

jkaflik commented Feb 13, 2023

What happened:

Currently, the user is forced to follow the predefined algorithm that figures out the expected data format based on fields available in the query and its name. (please see Screenshots section)

This requires the user to remember the rules to follow described in README.md.

What you expected to happen:

The user should be able to intentionally select what is the data format expected by him. Please see the screenshot attached from a view in Altinity's CH data source plugin.

Screenshots

Grafana CH plugin:

export const getFormat = (sql: string): Format => {
  // convention to format as time series
  // first field as "time" alias and requires at least 2 fields (time and metric)
  const selectList = getFields(sql);
  // if there are more than 2 fields, index 1 will be a ','
  if (selectList.length > 2 && isString(selectList[0])) {
    const firstProjection = selectList[0].trim().toLowerCase();
    if (firstProjection.endsWith('as time')) {
      return Format.TIMESERIES;
    }
    if (firstProjection.endsWith('as log_time')) {
      return Format.LOGS;
    }
  }
  return Format.TABLE;
};

Altinity's data source plugin:
Screenshot 2023-02-13 at 16 48 21

@jkaflik jkaflik added enhancement New feature or request datasource/ClickHouse labels Feb 13, 2023
@asimpson
Copy link
Contributor

This is a UX problem in my opinion. If the user selects "Time series" but the returned data doesn't contain the right fields for that format do we throw an error? Do we switch back to Table view? Both? The current reliance on user's having stored the README rules in their brains isn't ideal either.

We could do something where we have the three formats as a dropdown in the header of the query builder and when you switch between them show some help text for each one? Ideally we work with UX on how this works.

@jkaflik
Copy link
Collaborator Author

jkaflik commented Feb 20, 2023

I suggest providing a dropdown in the header so that user can pick format in advance. Based on selection, there might be either a info box displayed with details on query requirements, or we can be more proactive validating whether query does have everything we need. (not sure if doable as user can provide his query)
When data frame received from backend, fields has to be validated.

@asimpson
Please let me know if you want to follow up on it with UX on your side.

@asimpson
Copy link
Contributor

@jkaflik I'll post back here once I have a chat with UX. The next step after that will be figuring out when this can get worked on.

@asimpson asimpson self-assigned this Feb 27, 2023
@asimpson
Copy link
Contributor

asimpson commented Mar 3, 2023

Hopefully I'll be able to reconnect with UX next week. Sorry for the delay.

@mshustov
Copy link
Collaborator

mshustov commented Mar 3, 2023

Other things we can do based on the query selector:

@jkaflik
Copy link
Collaborator Author

jkaflik commented Mar 20, 2023

Hi @asimpson

did you had a chance to speak to UX?

@gingerwizard
Copy link
Collaborator

We need this for the traces view as well which enforces a similar req. https://grafana.com/docs/grafana/latest/explore/trace-integration/#data-api

@asimpson
Copy link
Contributor

@jkaflik I have not, I will try to chat with them Wednesday.

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

Successfully merging a pull request may close this issue.

4 participants