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

Plugins: Add deprecation notice for /api/tsdb/query endpoint #45238

Merged
merged 29 commits into from
Mar 30, 2022

Conversation

wbrowne
Copy link
Member

@wbrowne wbrowne commented Feb 10, 2022

What this PR does / why we need it:
This PR ensures references to /api/tsdb/query are marked as deprecated, and includes recommendations to use the newer /api/ds/query in its place.

Deprecation notice

/api/tsdb/query API has been deprecated and will be removed in a future release. Use /api/ds/query instead.

@wbrowne wbrowne added this to the 8.4.0 milestone Feb 10, 2022
@wbrowne wbrowne added the backport v8.4.x Mark PR for automatic backport to v8.4.x label Feb 10, 2022
@wbrowne wbrowne self-assigned this Feb 10, 2022
@wbrowne wbrowne modified the milestones: 8.4.0, 8.4.1 Feb 15, 2022
@grafanabot grafanabot removed this from the 8.4.1 milestone Feb 18, 2022
@grafanabot
Copy link
Contributor

This pull request was removed from the 8.4.1 milestone because 8.4.1 is currently being released.

@wbrowne wbrowne requested a review from marefr February 28, 2022 11:29
Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

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

LGTM, but added some comments/suggestions.

Remember to add a deprecation notice in the PR description, see instructions in Merge a pull request.

docs/sources/http_api/data_source.md Outdated Show resolved Hide resolved
docs/sources/http_api/data_source.md Outdated Show resolved Hide resolved
docs/sources/http_api/data_source.md Outdated Show resolved Hide resolved
docs/sources/http_api/data_source.md Outdated Show resolved Hide resolved
pkg/api/api.go Show resolved Hide resolved
@@ -200,13 +200,17 @@ import (
//
// Query metrics.
//
// Please refer to [updated API](#/ds/queryMetricsWithExpressions) instead
Copy link
Member

Choose a reason for hiding this comment

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

I think that we might need to update the queryMetricsWithExpressions definition to make it output proper response example in open api spec because the data frames conversion to JSON is custom, but I might be wrong?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean when it returns encoded arrow? AFAIK everything is JSON nowadays https://github.com/grafana/grafana/blob/main/pkg/api/metrics.go#L98

Copy link
Member

Choose a reason for hiding this comment

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

Was referring to JSON, but haven't checked swagger or the generated open api spec so not sure. Mainly referring to this https://github.com/grafana/grafana-plugin-sdk-go/blob/a6e537820b7b4554637de588e87ca2d77b8cad90/backend/data.go#L74-L82 that writes the JSON in a custom way by iterating the responses/frames.

Copy link
Member

Choose a reason for hiding this comment

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

Checked using swagger now, http://localhost:3000/swagger-ui. This might be a bigger problem than just this endpoint and potentially something we might attack later. But here it goes.

The request body example includes the following:

{
  "debug": true,
  "from": "now-1h",
  "queries": [
    {
      "datasourceId": 86,
      "format": "table",
      "intervalMs": 86400000,
      "maxDataPoints": 1092,
      "rawSql": "SELECT 1 as valueOne, 2 as valueTwo",
      "refId": "A"
    }
  ],
  "to": "now"
}

The 200 response includes the following:

Details
{
  "Responses": {
    "additionalProp1": {
      "message": "string",
      "results": {
        "additionalProp1": {
          "dataframes": {},
          "error": "string",
          "meta": {},
          "refId": "string",
          "series": [
            {
              "name": "string",
              "points": [
                [
                  {
                    "Float64": 0,
                    "Valid": true
                  }
                ]
              ],
              "tags": {
                "additionalProp1": "string",
                "additionalProp2": "string",
                "additionalProp3": "string"
              }
            }
          ],
          "tables": [
            {
              "columns": [
                {
                  "text": "string"
                }
              ],
              "rows": [
                [
                  {}
                ]
              ]
            }
          ]
        },
        "additionalProp2": {
          "dataframes": {},
          "error": "string",
          "meta": {},
          "refId": "string",
          "series": [
            {
              "name": "string",
              "points": [
                [
                  {
                    "Float64": 0,
                    "Valid": true
                  }
                ]
              ],
              "tags": {
                "additionalProp1": "string",
                "additionalProp2": "string",
                "additionalProp3": "string"
              }
            }
          ],
          "tables": [
            {
              "columns": [
                {
                  "text": "string"
                }
              ],
              "rows": [
                [
                  {}
                ]
              ]
            }
          ]
        },
        "additionalProp3": {
          "dataframes": {},
          "error": "string",
          "meta": {},
          "refId": "string",
          "series": [
            {
              "name": "string",
              "points": [
                [
                  {
                    "Float64": 0,
                    "Valid": true
                  }
                ]
              ],
              "tags": {
                "additionalProp1": "string",
                "additionalProp2": "string",
                "additionalProp3": "string"
              }
            }
          ],
          "tables": [
            {
              "columns": [
                {
                  "text": "string"
                }
              ],
              "rows": [
                [
                  {}
                ]
              ]
            }
          ]
        }
      }
    },
    "additionalProp2": {
      "message": "string",
      "results": {
        "additionalProp1": {
          "dataframes": {},
          "error": "string",
          "meta": {},
          "refId": "string",
          "series": [
            {
              "name": "string",
              "points": [
                [
                  {
                    "Float64": 0,
                    "Valid": true
                  }
                ]
              ],
              "tags": {
                "additionalProp1": "string",
                "additionalProp2": "string",
                "additionalProp3": "string"
              }
            }
          ],
          "tables": [
            {
              "columns": [
                {
                  "text": "string"
                }
              ],
              "rows": [
                [
                  {}
                ]
              ]
            }
          ]
        },
        "additionalProp2": {
          "dataframes": {},
          "error": "string",
          "meta": {},
          "refId": "string",
          "series": [
            {
              "name": "string",
              "points": [
                [
                  {
                    "Float64": 0,
                    "Valid": true
                  }
                ]
              ],
              "tags": {
                "additionalProp1": "string",
                "additionalProp2": "string",
                "additionalProp3": "string"
              }
            }
          ],
          "tables": [
            {
              "columns": [
                {
                  "text": "string"
                }
              ],
              "rows": [
                [
                  {}
                ]
              ]
            }
          ]
        },
        "additionalProp3": {
          "dataframes": {},
          "error": "string",
          "meta": {},
          "refId": "string",
          "series": [
            {
              "name": "string",
              "points": [
                [
                  {
                    "Float64": 0,
                    "Valid": true
                  }
                ]
              ],
              "tags": {
                "additionalProp1": "string",
                "additionalProp2": "string",
                "additionalProp3": "string"
              }
            }
          ],
          "tables": [
            {
              "columns": [
                {
                  "text": "string"
                }
              ],
              "rows": [
                [
                  {}
                ]
              ]
            }
          ]
        }
      }
    },
    "additionalProp3": {
      "message": "string",
      "results": {
        "additionalProp1": {
          "dataframes": {},
          "error": "string",
          "meta": {},
          "refId": "string",
          "series": [
            {
              "name": "string",
              "points": [
                [
                  {
                    "Float64": 0,
                    "Valid": true
                  }
                ]
              ],
              "tags": {
                "additionalProp1": "string",
                "additionalProp2": "string",
                "additionalProp3": "string"
              }
            }
          ],
          "tables": [
            {
              "columns": [
                {
                  "text": "string"
                }
              ],
              "rows": [
                [
                  {}
                ]
              ]
            }
          ]
        },
        "additionalProp2": {
          "dataframes": {},
          "error": "string",
          "meta": {},
          "refId": "string",
          "series": [
            {
              "name": "string",
              "points": [
                [
                  {
                    "Float64": 0,
                    "Valid": true
                  }
                ]
              ],
              "tags": {
                "additionalProp1": "string",
                "additionalProp2": "string",
                "additionalProp3": "string"
              }
            }
          ],
          "tables": [
            {
              "columns": [
                {
                  "text": "string"
                }
              ],
              "rows": [
                [
                  {}
                ]
              ]
            }
          ]
        },
        "additionalProp3": {
          "dataframes": {},
          "error": "string",
          "meta": {},
          "refId": "string",
          "series": [
            {
              "name": "string",
              "points": [
                [
                  {
                    "Float64": 0,
                    "Valid": true
                  }
                ]
              ],
              "tags": {
                "additionalProp1": "string",
                "additionalProp2": "string",
                "additionalProp3": "string"
              }
            }
          ],
          "tables": [
            {
              "columns": [
                {
                  "text": "string"
                }
              ],
              "rows": [
                [
                  {}
                ]
              ]
            }
          ]
        }
      }
    }
  }
}

@wbrowne wbrowne modified the milestones: 8.4.4, 8.5.0 Mar 11, 2022
@wbrowne wbrowne removed this from the 8.5.0 milestone Mar 22, 2022
@wbrowne wbrowne added no-backport Skip backport of PR and removed backport v8.4.x Mark PR for automatic backport to v8.4.x labels Mar 22, 2022
@wbrowne wbrowne added this to the 8.5.0 milestone Mar 22, 2022
@wbrowne wbrowne requested a review from marefr March 24, 2022 12:55
@wbrowne wbrowne changed the title [WIP] Plugins: Add deprecation notice for /api/tsdb/query endpoint Plugins: Add deprecation notice for /api/tsdb/query endpoint Mar 24, 2022
Copy link
Member

@marefr marefr left a comment

Choose a reason for hiding this comment

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

LGTM added some minor comments/suggestions

docs/sources/http_api/data_source.md Show resolved Hide resolved
docs/sources/http_api/data_source.md Show resolved Hide resolved
Co-authored-by: Marcus Efraimsson <marcus.efraimsson@gmail.com>
Copy link
Contributor

@achatterjee-grafana achatterjee-grafana left a comment

Choose a reason for hiding this comment

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

Added doc review.

docs/sources/http_api/data_source.md Outdated Show resolved Hide resolved
docs/sources/http_api/data_source.md Outdated Show resolved Hide resolved
docs/sources/http_api/data_source.md Outdated Show resolved Hide resolved
docs/sources/http_api/data_source.md Outdated Show resolved Hide resolved
docs/sources/http_api/data_source.md Show resolved Hide resolved
docs/sources/http_api/data_source.md Outdated Show resolved Hide resolved
docs/sources/http_api/data_source.md Outdated Show resolved Hide resolved
@wbrowne
Copy link
Member Author

wbrowne commented Mar 24, 2022

Thanks for the review @achatterjee-grafana!

FYI I have actually decided to remove the explicit annotations for (Optional) and (Required) as it can be misleading as the requests totally depend on the data source implementation.

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

Successfully merging this pull request may close these issues.

None yet

4 participants