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

[RUM Dashboard] Visitors by region map #77135

Merged
merged 26 commits into from
Sep 14, 2020
Merged

Conversation

shahzad31
Copy link
Contributor

@shahzad31 shahzad31 commented Sep 10, 2020

Summary

Fixes: #72161

Added a visitor by region page load duration map

image

@shahzad31 shahzad31 marked this pull request as ready for review September 10, 2020 12:01
@shahzad31 shahzad31 requested review from a team as code owners September 10, 2020 12:01
@shahzad31 shahzad31 self-assigned this Sep 10, 2020
@shahzad31 shahzad31 added release_note:enhancement v7.10.0 v8.0.0 Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability labels Sep 10, 2020
@formgeist
Copy link
Contributor

@shahzad31 Would it be possible to add units to the data in the layers panel and possibly apply formats to correctly set the unit?

Screenshot 2020-09-10 at 14 21 24

@@ -28,7 +33,11 @@ export type LayerDescriptor = {
areLabelsOnTop?: boolean;
minZoom?: number;
maxZoom?: number;
sourceDescriptor: AbstractSourceDescriptor | null;
sourceDescriptor:
Copy link
Contributor

@nreese nreese Sep 10, 2020

Choose a reason for hiding this comment

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

The maps plugin exposes a registerSource method, allowing plugins to register custom sources. Prior to custom sources, sourceDescriptor used to be the union of all source descriptor types. Now, the typing only contains the minimum required fields.

Where is this causing typing concerns? Can you cast your sourceDescriptor to add more typing specificity?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverted this change

@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Sep 10, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/apm-ui (Team:apm)

Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

This looks awesome. I did have a question whether these geo selection filters are working, they didn't seem to operate correctly when I tried them out. I'm still performing the rest of the code review, but I wanted to get you this feedback in the meantime.

image

@shahzad31 shahzad31 requested a review from nreese September 10, 2020 15:29
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

maps changes LGTM
code review

@drewpost
Copy link

@shahzad31 on the page load duration view in map, what units am I looking at here? I'd expect to see seconds.

Copy link
Contributor

@justinkambic justinkambic left a comment

Choose a reason for hiding this comment

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

LGTM!

@shahzad31
Copy link
Contributor Author

@elasticmachine merge upstream

STYLE_TYPE,
SYMBOLIZE_AS_TYPES,
} from '../../../../../../maps/common/constants';
import { APM_STATIC_INDEX_PATTERN_ID } from '../../../../../common/apm_index_pattern';
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary to duplicate this file instead of importing from apm_oss?

* you may not use this file except in compliance with the Elastic License.
*/

export const APM_STATIC_INDEX_PATTERN_ID = 'apm_static_index_pattern_id';
Copy link
Member

Choose a reason for hiding this comment

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

"requiredBundles": [
"kibanaReact",
"kibanaUtils",
"observability",
"home"
"home",
"maps"
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps add a comment that this is used by csm. This way we can remove it when csm is moved to a separate app.

Copy link
Contributor

Choose a reason for hiding this comment

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

JSON doesn't have comments.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

@kbn/optimizer bundle module count

id value diff baseline
apm 1249 +10 1239

async chunks size

id value diff baseline
apm 4.9MB +23.2KB 4.9MB
maps 3.3MB -2.9KB 3.3MB
total +20.3KB

page load bundle size

id value diff baseline
apm 42.3KB +538.0B 41.7KB
maps 300.9KB +3.1KB 297.8KB
total +3.7KB

History

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

@shahzad31 shahzad31 merged commit dcd119c into elastic:master Sep 14, 2020
@shahzad31 shahzad31 deleted the visitor-map branch September 14, 2020 16:22
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 14, 2020
* master: (65 commits)
  [Security Solution][Resolver] Analyzed event styling (elastic#77115)
  filter invalid SOs from the searc hresults in Task Manager (elastic#76891)
  [RUM Dashboard] Visitors by region map (elastic#77135)
  [Security Solution][Endpoint][Admin] Task/endpoint list actions (elastic#76555)
  [Ingest pipelines] Forms for processors T-U (elastic#76710)
  updating datatable type (elastic#77320)
  [ML] Fix custom URLs processing for security app (elastic#76957)
  [telemetry] add schema guideline + schema_check new check for --path config (elastic#75747)
  [ML] Transforms: API schemas and integration tests (elastic#75164)
  [Mappings editor] Add support for wildcard field type (elastic#76574)
  [Ingest Manager] Fix flyout instruction selection (elastic#77071)
  [Telemetry Tools] update lodash to 4.17 (elastic#77317)
  [APM] Service inventory redesign (elastic#76744)
  Hide management sections based on cluster/index privileges (elastic#67791)
  [Snapshot Restore] Disable steps when form is invalid (elastic#76540)
  [Mappings editor] Add support for positive_score_impact to rank_feature (elastic#76824)
  Update apm.ts (elastic#77310)
  [OBS] Remove beta badge, change news feed size and add external icon to news feed link (elastic#77164)
  [Discover] Convert legacy sort to be compatible with multi sort (elastic#76986)
  [APM] API Snapshot Testing (elastic#77229)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Sep 14, 2020
* master: (65 commits)
  [Security Solution][Resolver] Analyzed event styling (elastic#77115)
  filter invalid SOs from the searc hresults in Task Manager (elastic#76891)
  [RUM Dashboard] Visitors by region map (elastic#77135)
  [Security Solution][Endpoint][Admin] Task/endpoint list actions (elastic#76555)
  [Ingest pipelines] Forms for processors T-U (elastic#76710)
  updating datatable type (elastic#77320)
  [ML] Fix custom URLs processing for security app (elastic#76957)
  [telemetry] add schema guideline + schema_check new check for --path config (elastic#75747)
  [ML] Transforms: API schemas and integration tests (elastic#75164)
  [Mappings editor] Add support for wildcard field type (elastic#76574)
  [Ingest Manager] Fix flyout instruction selection (elastic#77071)
  [Telemetry Tools] update lodash to 4.17 (elastic#77317)
  [APM] Service inventory redesign (elastic#76744)
  Hide management sections based on cluster/index privileges (elastic#67791)
  [Snapshot Restore] Disable steps when form is invalid (elastic#76540)
  [Mappings editor] Add support for positive_score_impact to rank_feature (elastic#76824)
  Update apm.ts (elastic#77310)
  [OBS] Remove beta badge, change news feed size and add external icon to news feed link (elastic#77164)
  [Discover] Convert legacy sort to be compatible with multi sort (elastic#76986)
  [APM] API Snapshot Testing (elastic#77229)
  ...
shahzad31 added a commit to shahzad31/kibana that referenced this pull request Sep 14, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
shahzad31 added a commit that referenced this pull request Sep 15, 2020
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
@sorenlouv sorenlouv removed the Team:APM All issues that need APM UI Team support label Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:enhancement Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.10.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Uptime] Add map to RUM visitor breakdown dashboard
9 participants