Skip to content

[inspector] show request method, path, and querystring#166565

Merged
nreese merged 30 commits intoelastic:mainfrom
nreese:issue_45931
Sep 21, 2023
Merged

[inspector] show request method, path, and querystring#166565
nreese merged 30 commits intoelastic:mainfrom
nreese:issue_45931

Conversation

@nreese
Copy link
Contributor

@nreese nreese commented Sep 15, 2023

Closes #45931

PR updates bsearch service to return request params from elasticsearch-js client requests. This provides inspector with the exact details used to fetch data from elasticsearch, ensuring inspector displays request exactly as used by elasticsearch-js client.

ESQL This PR makes it possible to open ESQL searches in console.
Screen Shot 2023-09-16 at 4 19 58 PM

@nreese
Copy link
Contributor Author

nreese commented Sep 18, 2023

@elasticmachine merge upstream

@kibana-ci
Copy link

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
inspector 40 41 +1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
kibanaUtils 417 419 +2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
inspector 16.0KB 16.2KB +203.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
data 407.5KB 407.7KB +205.0B
inspector 21.7KB 22.1KB +414.0B
total +619.0B
Unknown metric groups

API count

id before after diff
data 3308 3310 +2
kibanaUtils 610 612 +2
total +4

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@nreese nreese added release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v8.11.0 Feature:Inspector Inspector infrastructure and implementations labels Sep 18, 2023
@nreese nreese marked this pull request as ready for review September 18, 2023 23:32
@nreese nreese requested review from a team as code owners September 18, 2023 23:32
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-presentation (Team:Presentation)

Copy link
Contributor

@ppisljar ppisljar left a comment

Choose a reason for hiding this comment

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

code LGTM

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

This is a huge enhancement not only for ES|QL but in general. I tested it in Chrome for ESQL in Discover, ESQL charts, Lens charts (made with dataviews) and works great. LGTM!

@ThomThomson ThomThomson self-requested a review September 19, 2023 15:48
Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

LGTM! Looked through the code and ran locally. The inspector is way more useful like this. Nice change! Left one small question

Copy link
Contributor

@jughosta jughosta left a comment

Choose a reason for hiding this comment

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

Great work! 👍

@nreese nreese merged commit d61a5a0 into elastic:main Sep 21, 2023
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Sep 21, 2023
rudolf added a commit to rudolf/kibana that referenced this pull request Sep 28, 2023
delanni pushed a commit that referenced this pull request Sep 28, 2023
)

This reverts commit d61a5a0 from
#166565

## Summary

Summarize your PR. If it involves visual changes include a screenshot or
gif.


### Checklist

Delete any items that are not applicable to this PR.

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [ ]
[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)
was added for features that require explanation or tutorials
- [ ] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [ ] Any UI touched in this PR is usable by keyboard only (learn more
about [keyboard accessibility](https://webaim.org/techniques/keyboard/))
- [ ] Any UI touched in this PR does not create any new axe failures
(run axe in browser:
[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),
[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))
- [ ] If a plugin configuration key changed, check if it needs to be
allowlisted in the cloud and added to the [docker
list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)
- [ ] This renders correctly on smaller devices using a responsive
layout. (You can test this [in your
browser](https://www.browserstack.com/guide/responsive-testing-on-local-server))
- [ ] This was checked for [cross-browser
compatibility](https://www.elastic.co/support/matrix#matrix_browsers)


### Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to
identify risks that should be tested prior to the change/feature
release.

When forming the risk matrix, consider some of the following examples
and how they may potentially impact the change:

| Risk | Probability | Severity | Mitigation/Notes |

|---------------------------|-------------|----------|-------------------------|
| Multiple Spaces—unexpected behavior in non-default Kibana Space.
| Low | High | Integration tests will verify that all features are still
supported in non-default Kibana Space and when user switches between
spaces. |
| Multiple nodes—Elasticsearch polling might have race conditions
when multiple Kibana nodes are polling for the same tasks. | High | Low
| Tasks are idempotent, so executing them multiple times will not result
in logical error, but will degrade performance. To test for this case we
add plenty of unit tests around this logic and document manual testing
procedure. |
| Code should gracefully handle cases when feature X or plugin Y are
disabled. | Medium | High | Unit tests will verify that any feature flag
or plugin combination still results in our service operational. |
| [See more potential risk
examples](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) |


### For maintainers

- [ ] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
nreese added a commit that referenced this pull request Nov 3, 2023
Closes #45931

PR updates data plugin `search` and `bsearch` endpoints to return
method, path, and querystring request params from elasticsearch-js
client requests. This provides inspector with the exact details used to
fetch data from elasticsearch, ensuring inspector displays request
exactly as used by elasticsearch-js client.

**ESQL** This PR makes it possible to open ESQL searches in console.
<img width="500" alt="Screen Shot 2023-09-16 at 4 19 58 PM"
src="https://github.com/elastic/kibana/assets/373691/56019fb5-ca88-46cf-a42f-86f5f51edfcc">

### background
If you are thinking to yourself, "haven't I reviewed this before?", you
are right. This functionality has been through several iterations.
1) Original PR #166565 was
reverted for exposing `headers`.
2) [Fix to only expose method, path, and querystring keys from request
parameters](#167544) was rejected
because it applied changes to
`kibana_utils/server/report_server_error.ts`, which is used extensively
throughout Kibana.
3) This iteration moves logic into the data plugin to be as narrow as
possible.

---------

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Inspector Inspector infrastructure and implementations release_note:enhancement Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas t// v8.11.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inspector should show request endpoint as well as body

8 participants