[APM] Average page load duration by country chart #43567
Conversation
This comment has been minimized.
This comment has been minimized.
💔 Build Failed |
...egacy/plugins/apm/public/components/shared/charts/TransactionCharts/RegionMapChart/index.tsx
Outdated
Show resolved
Hide resolved
...egacy/plugins/apm/public/components/shared/charts/TransactionCharts/RegionMapChart/index.tsx
Outdated
Show resolved
Hide resolved
...egacy/plugins/apm/public/components/shared/charts/TransactionCharts/RegionMapChart/index.tsx
Outdated
Show resolved
Hide resolved
| return ( | ||
| <div style={{ textAlign: 'center' }}> | ||
| <div style={{ fontSize: 'larger' }}>{geojsonProperties.name}</div> | ||
| <div>Avg. page load duration:</div> |
...egacy/plugins/apm/public/components/shared/charts/TransactionCharts/RegionMapChart/index.tsx
Outdated
Show resolved
Hide resolved
...egacy/plugins/apm/public/components/shared/charts/TransactionCharts/RegionMapChart/index.tsx
Outdated
Show resolved
Hide resolved
x-pack/legacy/plugins/apm/public/hooks/useAvgDurationByCountry.ts
Outdated
Show resolved
Hide resolved
x-pack/legacy/plugins/apm/public/services/rest/apm/transaction_groups.ts
Outdated
Show resolved
Hide resolved
...egacy/plugins/apm/public/components/shared/charts/TransactionCharts/RegionMapChart/index.tsx
Outdated
Show resolved
Hide resolved
...apm/public/components/shared/charts/TransactionCharts/RegionMapChart/ColorProgressionBar.tsx
Outdated
Show resolved
Hide resolved
| @@ -13,3 +13,6 @@ bluebird.Promise.setScheduler(function (fn) { global.setImmediate.call(global, f | |||
|
|
|||
| const MutationObserver = require('mutation-observer'); | |||
| Object.defineProperty(window, 'MutationObserver', { value: MutationObserver }); | |||
|
|
|||
| const URL = { createObjectURL: () => '' }; | |||
There was a problem hiding this comment.
Why do we need this? (Maybe good to leave a comment here anyway).
There was a problem hiding this comment.
It's necessary since we have tests which attempt to render the map in jsdom in tests. window.URL.createObjectUrl is called when initializing mapbox, so this is added to not break tests.
|
Tooltip looks strange in Chrome/OS X:
|
| const hoveredFeaturePropertiesRef = useRef< | ||
| mapboxgl.MapboxGeoJSONFeature['properties'] | ||
| >(null); | ||
| const isMapReady = useRef<boolean>(false); |
There was a problem hiding this comment.
Could you explain why you're using useRef instead of useState/useCallback? I've got a feeling this code would be simpler and more robust if we avoid useRef.
There was a problem hiding this comment.
You're right that useState should be generally preferred over useRef hooks. In the latest commit, i removed most of the useRef, and replaced them with more useCallbacks for event handlers, but but it still requires a useRef pointer to certain callbacks. This is because we're wrapping an evented object with a react view component. The initial useEffect which instantiates everything also sets up event handlers, which should not change between renders. The initial useEffect also references scoped values from that first render in the lifecycle available to the js closure of it's initial execution context.
So this is one of the cases where we must make use of useRef if we want to use functional components. If it was a class-based component, they would exist as instance variables, populated in the constructor.
| if (currentLayer) { | ||
| map.removeLayer(choroplethLayerId.current); | ||
| } | ||
| const symbolLayer = (map.getStyle().layers || []).find( |
There was a problem hiding this comment.
The symbol layer comes along with the vector tileset from the Elastic Maps Service. We need to reference it so that we can place our custom layer before the symbol layer so it doesn't obscure labels in the map.
...ugins/apm/public/components/shared/charts/TransactionCharts/RegionMapChart/ChoroplethMap.tsx
Outdated
Show resolved
Hide resolved
...ugins/apm/public/components/shared/charts/TransactionCharts/RegionMapChart/ChoroplethMap.tsx
Outdated
Show resolved
Hide resolved
...ugins/apm/public/components/shared/charts/TransactionCharts/RegionMapChart/ChoroplethMap.tsx
Outdated
Show resolved
Hide resolved
|
Like @dgieselaar I think we should only make the properties we actually use configurable. I'm okay with hardcoding the rest (as constants) if that will simplify the implementation. |
ogupte
force-pushed
the
apm-35571-rum-ui-region-chart
branch
from
ad24105
to
9e2735f
Compare
💔 Build Failed |
ogupte
force-pushed
the
apm-35571-rum-ui-region-chart
branch
from
9e2735f
to
bb892e3
Compare
|
I'm seeing this odd behaviour when hovering over the map: |
|
Tooltips The tooltips can be a little hard to read when the text overlaps with continent names and such. I think adding a solid background color (white) would be good.
|
|
Legend I think the legend values should be rounded In this case the viz is based on a single doc. It's a bit of an edge case |
ogupte
force-pushed
the
apm-35571-rum-ui-region-chart
branch
from
b988781
to
0c618b0
Compare
💚 Build Succeeded |
💚 Build Succeeded |
- fixed issue with map rendering multiple times - renamed initial props to start with 'initial' - added some more code comments
- check that a layer exists before querying features
- cleans up mouseover handler, which updates on state changes - formats doc count for display - style improvements
- Only render tooltip when there's data
- add better typing around Geojson propertier for world countries
ogupte
force-pushed
the
apm-35571-rum-ui-region-chart
branch
from
3445711
to
29bef0f
Compare
|
@elasticmachine run elasticsearch-ci/docs |
💚 Build Succeeded |
* [APM] Adds chart for page load averages by country in RUM page-load view * [APM] Simplified and refined ChoroplethMap. Added legend labels. * - Replaced Map legend slices with smooth gradient - fixed issue with map rendering multiple times - renamed initial props to start with 'initial' - added some more code comments * use correct i18n ids * - base color progression calc directly on euiColorPrimary - check that a layer exists before querying features * Addressed code review feedback * - fixes issue where min/max was not a finite value - cleans up mouseover handler, which updates on state changes - formats doc count for display - style improvements * addressed PR feedback & updated renovate.json5 * - Removed the Legend from the ChoroplethMap - Only render tooltip when there's data * - fix hover state not clearing properly - add better typing around Geojson propertier for world countries * added missing css import
* [APM] Adds chart for page load averages by country in RUM page-load view * [APM] Simplified and refined ChoroplethMap. Added legend labels. * - Replaced Map legend slices with smooth gradient - fixed issue with map rendering multiple times - renamed initial props to start with 'initial' - added some more code comments * use correct i18n ids * - base color progression calc directly on euiColorPrimary - check that a layer exists before querying features * Addressed code review feedback * - fixes issue where min/max was not a finite value - cleans up mouseover handler, which updates on state changes - formats doc count for display - style improvements * addressed PR feedback & updated renovate.json5 * - Removed the Legend from the ChoroplethMap - Only render tooltip when there's data * - fix hover state not clearing properly - add better typing around Geojson propertier for world countries * added missing css import
* [APM] Adds chart for page load averages by country in RUM page-load view * [APM] Simplified and refined ChoroplethMap. Added legend labels. * - Replaced Map legend slices with smooth gradient - fixed issue with map rendering multiple times - renamed initial props to start with 'initial' - added some more code comments * use correct i18n ids * - base color progression calc directly on euiColorPrimary - check that a layer exists before querying features * Addressed code review feedback * - fixes issue where min/max was not a finite value - cleans up mouseover handler, which updates on state changes - formats doc count for display - style improvements * addressed PR feedback & updated renovate.json5 * - Removed the Legend from the ChoroplethMap - Only render tooltip when there's data * - fix hover state not clearing properly - add better typing around Geojson propertier for world countries * added missing css import
* [APM] Adds chart for page load averages by country in RUM page-load view * [APM] Simplified and refined ChoroplethMap. Added legend labels. * - Replaced Map legend slices with smooth gradient - fixed issue with map rendering multiple times - renamed initial props to start with 'initial' - added some more code comments * use correct i18n ids * - base color progression calc directly on euiColorPrimary - check that a layer exists before querying features * Addressed code review feedback * - fixes issue where min/max was not a finite value - cleans up mouseover handler, which updates on state changes - formats doc count for display - style improvements * addressed PR feedback & updated renovate.json5 * - Removed the Legend from the ChoroplethMap - Only render tooltip when there's data * - fix hover state not clearing properly - add better typing around Geojson propertier for world countries * added missing css import
Adds the map chart from #35571
Can test locally by generating interesting geo data for the opbeans-rum app:
Use data generation tool at https://github.com/ogupte/apm-ui-seed
apm-ui-seed-geo --host='localhost:9200' --index='apm-8.0.0-transaction' --service-name='client' clean seed list. Then view page-loads for a RUM service.