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

[Infra Monitoring UI] update metrics-explorer-view interfaces to have strict types for validation #157520

Closed
neptunian opened this issue May 12, 2023 · 3 comments · Fixed by #160982
Assignees
Labels
Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.10.0

Comments

@neptunian
Copy link
Contributor

neptunian commented May 12, 2023

📓 Summary

As part of the road to versioned HTTP APIs:

3. HTTP APIs have strict input validation

the API should not accept any data that could cause application code to throw exceptions or otherwise fail. E.g. given an input field value. If your application code divides by value you need to add value != 0 validation or explicitly handle that case.

The metrics-explorer-view so-attribs interface does not type all the necessary fields that are required for the app to function properly nor does the HTTP API interface.

Eg: posting to API with a number instead of a string in options.metrics.field

POST http://localhost:5601/api/infra/metrics-explorer-view
{
  "attributes": {
        "options": {
            "aggregation": "avg",
            "metrics": [
                {
                    "aggregation": "avg",
                    "field": 1,
                    "color": "color0"
                }
            ],
            "source": "default"
        },
...
}

Response status: 201 created

App after navigating to new saved view:
Screenshot 2023-05-12 at 12 39 25 PM

✔️ Acceptance criteria

  • The /api/infra/metrics-explorer-view route should have strict input validation
  • Loading a view should not cause the app to fail because of some input

💡 Notes

  • Updating the runtime types should be sufficient in resolving this issue
  • so-attribs interface and HTTP API interface must remain separate and should not refer to each other even if they are the same
@neptunian neptunian added Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services labels May 12, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/infra-monitoring-ui (Team:Infra Monitoring UI)

@roshan-elastic
Copy link

@neptunian - checking whether this would make sense to do in 8.10? I moved this to the top of the 'ready' column - I can label as 8.10 to give it a bit more visibility.

@neptunian
Copy link
Contributor Author

Sure. It has no dependencies and should be a straightforward task.

@crespocarlos crespocarlos self-assigned this Jun 29, 2023
crespocarlos added a commit that referenced this issue Jul 14, 2023
…_views endpoint (#160982)

closes [#157520](#157520)
## Summary

This PR adds strict payload validation to `metrics_explorer_views`
endpoint. This PR depends on this to be merged
#160852


### How to test

- Call the endpoint below trying to use invalid values. see
[here](https://github.com/elastic/kibana/pull/160982/files#diff-4573683b3b62cdf5f6426ec345b7ad6c7d6e6328237b213ca7519f686d8fa951R125-R131).

```bash
[POST|PUT] kbn:/api/infra/metrics_explorer_views
{
  "attributes": {
    "name": "Ad-hoc",
    "options": {
      "aggregation": "avg",
      "metrics": [
        {
          "aggregation": "avg",
          "field": "system.cpu.total.norm.pct",
          "color": "color0"
        },
      ],
      "source": "default",
      "groupBy": [
        "host.name"
      ]
    },
    "chartOptions": {
      "type": "line",
      "yAxisMode": "fromZero",
      "stack": false
    },
    "currentTimerange": {
      "from": "now-1h",
      "to": "now",
      "interval": ">=10s"
    }
  }
}
```

- Set up a local Kibana instance
- Navigate to `Infrastructure > Metrics Explorer`
- In the UI, use the Saved View feature and try different field
combinations

---------

Co-authored-by: Kibana Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Metrics UI Metrics UI feature Team:Infra Monitoring UI - DEPRECATED DEPRECATED - Label for the Infra Monitoring UI team. Use Team:obs-ux-infra_services v8.10.0
Projects
None yet
4 participants