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

[maps] include vector tile layers in geoShapeAggLayersCount telemetry #151072

Merged
merged 2 commits into from
Feb 13, 2023

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Feb 13, 2023

While investigating #151064, I found a problem with IndexPatternStatsCollector where geo_shape aggregation usage with vector tile layers are not counted.

Steps to view problem:

  • Download world countries geojson
  • use file upload to upload world countries into your Elastic stack
  • add a new cluster layer to your map.
    • Select world countries index
    • Select Hexagons
    • Click Add layer
    • Save map
  • Open borwser dev tools and switch to network tab
  • Open Kibana dev tools and run
    POST kbn:api/telemetry/v2/clusters/_stats
    { "unencrypted": true }
    
  • Copy response for _stats request. Search for geoShapeAggLayersCount. Notice how the value is zero but it should be one since you have one map using geo shape aggregation
    Screen Shot 2023-02-13 at 1 14 34 PM

PR resolves the problem by removing layer type guard. The guard is error prone and easy to not update with new layer types. The guard does not provide any value, since the logic is really concerned with source types and the source type guards provide the correct protections.

Steps to test:
Follow steps above and verify geoShapeAggLayersCount is one

@nreese nreese added Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas release_note:skip Skip the PR/issue when compiling release notes auto-backport Deprecated: Automatically backport this PR after it's merged Feature:Maps v8.7.0 v8.8.0 labels Feb 13, 2023
@nreese nreese marked this pull request as ready for review February 13, 2023 20:13
@nreese nreese requested a review from a team as a code owner February 13, 2023 20:13
@elasticmachine
Copy link
Contributor

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

@nreese nreese changed the title [maps] include vector layers in geoShapeAggLayersCount telemetry [maps] include vector tile layers in geoShapeAggLayersCount telemetry Feb 13, 2023
Copy link
Member

@nickpeihl nickpeihl left a comment

Choose a reason for hiding this comment

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

lgtm! thanks for explaining why the type guard is unnecessary.

code review and tested in chrome

@kibana-ci
Copy link
Collaborator

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #51 / Discover alerting Search source Alert should display actual data view state after update on clicking viewInApp link
  • [job] [logs] FTR Configs #51 / Discover alerting Search source Alert should display actual state after rule params update on clicking viewInApp link
  • [job] [logs] FTR Configs #51 / Discover alerting Search source Alert should display prev rule state after params update on clicking prev generated link

Metrics [docs]

✅ unchanged

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

@nreese nreese merged commit 9d7e109 into elastic:main Feb 13, 2023
kibanamachine pushed a commit that referenced this pull request Feb 13, 2023
…#151072)

While investigating #151064, I
found a problem with IndexPatternStatsCollector where geo_shape
aggregation usage with vector tile layers are not counted.

Steps to view problem:
* Download [world countries
geojson](https://vector.maps.elastic.co/files/world_countries_v7.geo.json?elastic_tile_service_tos=agree&my_app_name=ems-landing-page&my_app_version=8.6.0&license=643c1faf-80fc-4ab0-9323-4d9bd11f4bbc)
* use file upload to upload world countries into your Elastic stack
* add a new cluster layer to your map.
    * Select world countries index
    * Select **Hexagons**
    * Click **Add layer**
    * Save map
* Open borwser dev tools and switch to network tab
* Open Kibana dev tools and run
    ```
    POST kbn:api/telemetry/v2/clusters/_stats
    { "unencrypted": true }
    ```
* Copy response for `_stats` request. Search for
`geoShapeAggLayersCount`. Notice how the value is zero but it should be
one since you have one map using geo shape aggregation
<img width="600" alt="Screen Shot 2023-02-13 at 1 14 34 PM"
src="https://user-images.githubusercontent.com/373691/218565153-0060dd4b-e422-477f-8b07-9f4dabd73064.png">

PR resolves the problem by removing layer type guard. The guard is error
prone and easy to not update with new layer types. The guard does not
provide any value, since the logic is really concerned with source types
and the source type guards provide the correct protections.

Steps to test:
Follow steps above and verify `geoShapeAggLayersCount` is one

(cherry picked from commit 9d7e109)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.7

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

kibanamachine added a commit that referenced this pull request Feb 14, 2023
…emetry (#151072) (#151089)

# Backport

This will backport the following commits from `main` to `8.7`:
- [[maps] include vector tile layers in geoShapeAggLayersCount telemetry
(#151072)](#151072)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Nathan
Reese","email":"reese.nathan@elastic.co"},"sourceCommit":{"committedDate":"2023-02-13T22:49:52Z","message":"[maps]
include vector tile layers in geoShapeAggLayersCount telemetry
(#151072)\n\nWhile investigating
#151064, I\r\nfound a problem
with IndexPatternStatsCollector where geo_shape\r\naggregation usage
with vector tile layers are not counted.\r\n\r\nSteps to view
problem:\r\n* Download [world
countries\r\ngeojson](https://vector.maps.elastic.co/files/world_countries_v7.geo.json?elastic_tile_service_tos=agree&my_app_name=ems-landing-page&my_app_version=8.6.0&license=643c1faf-80fc-4ab0-9323-4d9bd11f4bbc)\r\n*
use file upload to upload world countries into your Elastic stack\r\n*
add a new cluster layer to your map. \r\n * Select world countries
index\r\n * Select **Hexagons**\r\n * Click **Add layer**\r\n * Save
map\r\n* Open borwser dev tools and switch to network tab\r\n* Open
Kibana dev tools and run \r\n ```\r\n POST
kbn:api/telemetry/v2/clusters/_stats\r\n { \"unencrypted\": true }\r\n
```\r\n* Copy response for `_stats` request. Search
for\r\n`geoShapeAggLayersCount`. Notice how the value is zero but it
should be\r\none since you have one map using geo shape
aggregation\r\n<img width=\"600\" alt=\"Screen Shot 2023-02-13 at 1 14
34
PM\"\r\nsrc=\"https://user-images.githubusercontent.com/373691/218565153-0060dd4b-e422-477f-8b07-9f4dabd73064.png\">\r\n\r\n\r\nPR
resolves the problem by removing layer type guard. The guard is
error\r\nprone and easy to not update with new layer types. The guard
does not\r\nprovide any value, since the logic is really concerned with
source types\r\nand the source type guards provide the correct
protections.\r\n\r\nSteps to test:\r\nFollow steps above and verify
`geoShapeAggLayersCount` is
one","sha":"9d7e1095365483ad9bc083829f3f291c478e7cf6","branchLabelMapping":{"^v8.8.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["Team:Presentation","release_note:skip","auto-backport","Feature:Maps","v8.7.0","v8.8.0"],"number":151072,"url":"#151072
include vector tile layers in geoShapeAggLayersCount telemetry
(#151072)\n\nWhile investigating
#151064, I\r\nfound a problem
with IndexPatternStatsCollector where geo_shape\r\naggregation usage
with vector tile layers are not counted.\r\n\r\nSteps to view
problem:\r\n* Download [world
countries\r\ngeojson](https://vector.maps.elastic.co/files/world_countries_v7.geo.json?elastic_tile_service_tos=agree&my_app_name=ems-landing-page&my_app_version=8.6.0&license=643c1faf-80fc-4ab0-9323-4d9bd11f4bbc)\r\n*
use file upload to upload world countries into your Elastic stack\r\n*
add a new cluster layer to your map. \r\n * Select world countries
index\r\n * Select **Hexagons**\r\n * Click **Add layer**\r\n * Save
map\r\n* Open borwser dev tools and switch to network tab\r\n* Open
Kibana dev tools and run \r\n ```\r\n POST
kbn:api/telemetry/v2/clusters/_stats\r\n { \"unencrypted\": true }\r\n
```\r\n* Copy response for `_stats` request. Search
for\r\n`geoShapeAggLayersCount`. Notice how the value is zero but it
should be\r\none since you have one map using geo shape
aggregation\r\n<img width=\"600\" alt=\"Screen Shot 2023-02-13 at 1 14
34
PM\"\r\nsrc=\"https://user-images.githubusercontent.com/373691/218565153-0060dd4b-e422-477f-8b07-9f4dabd73064.png\">\r\n\r\n\r\nPR
resolves the problem by removing layer type guard. The guard is
error\r\nprone and easy to not update with new layer types. The guard
does not\r\nprovide any value, since the logic is really concerned with
source types\r\nand the source type guards provide the correct
protections.\r\n\r\nSteps to test:\r\nFollow steps above and verify
`geoShapeAggLayersCount` is
one","sha":"9d7e1095365483ad9bc083829f3f291c478e7cf6"}},"sourceBranch":"main","suggestedTargetBranches":["8.7"],"targetPullRequestStates":[{"branch":"8.7","label":"v8.7.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.8.0","labelRegex":"^v8.8.0$","isSourceBranch":true,"state":"MERGED","url":"#151072
include vector tile layers in geoShapeAggLayersCount telemetry
(#151072)\n\nWhile investigating
#151064, I\r\nfound a problem
with IndexPatternStatsCollector where geo_shape\r\naggregation usage
with vector tile layers are not counted.\r\n\r\nSteps to view
problem:\r\n* Download [world
countries\r\ngeojson](https://vector.maps.elastic.co/files/world_countries_v7.geo.json?elastic_tile_service_tos=agree&my_app_name=ems-landing-page&my_app_version=8.6.0&license=643c1faf-80fc-4ab0-9323-4d9bd11f4bbc)\r\n*
use file upload to upload world countries into your Elastic stack\r\n*
add a new cluster layer to your map. \r\n * Select world countries
index\r\n * Select **Hexagons**\r\n * Click **Add layer**\r\n * Save
map\r\n* Open borwser dev tools and switch to network tab\r\n* Open
Kibana dev tools and run \r\n ```\r\n POST
kbn:api/telemetry/v2/clusters/_stats\r\n { \"unencrypted\": true }\r\n
```\r\n* Copy response for `_stats` request. Search
for\r\n`geoShapeAggLayersCount`. Notice how the value is zero but it
should be\r\none since you have one map using geo shape
aggregation\r\n<img width=\"600\" alt=\"Screen Shot 2023-02-13 at 1 14
34
PM\"\r\nsrc=\"https://user-images.githubusercontent.com/373691/218565153-0060dd4b-e422-477f-8b07-9f4dabd73064.png\">\r\n\r\n\r\nPR
resolves the problem by removing layer type guard. The guard is
error\r\nprone and easy to not update with new layer types. The guard
does not\r\nprovide any value, since the logic is really concerned with
source types\r\nand the source type guards provide the correct
protections.\r\n\r\nSteps to test:\r\nFollow steps above and verify
`geoShapeAggLayersCount` is
one","sha":"9d7e1095365483ad9bc083829f3f291c478e7cf6"}}]}] BACKPORT-->

Co-authored-by: Nathan Reese <reese.nathan@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Deprecated: Automatically backport this PR after it's merged Feature:Maps release_note:skip Skip the PR/issue when compiling release notes Team:Presentation Presentation Team for Dashboard, Input Controls, and Canvas v8.7.0 v8.8.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants