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

[APM] Elastic chart issues #84238

Merged
merged 6 commits into from
Nov 25, 2020
Merged

Conversation

cauemarcondes
Copy link
Contributor

This PR solves a few issues described here: #84178

  • When browsing around and choosing different time ranges, the charts are still returning empty after initial displaying data. This makes for a very jarring experience 🔴🔴🔴🔴🔴
    ezgif com-gif-maker (2)

  • Y-axis ticks don't show maxValue or any other ticks for percentages

Screenshot 2020-11-24 at 16 45 36

@cauemarcondes cauemarcondes added release_note:skip Skip the PR/issue when compiling release notes v7.11.0 labels Nov 24, 2020
@cauemarcondes cauemarcondes requested a review from a team as a code owner November 24, 2020 15:50
@botelastic botelastic bot added the Team:APM All issues that need APM UI Team support label Nov 24, 2020
@elasticmachine
Copy link
Contributor

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

}}
marker={<EuiIcon type="dot" color={annotationColor} />}
markerPosition={Position.Top}
/>
Copy link
Member

@sorenlouv sorenlouv Nov 24, 2020

Choose a reason for hiding this comment

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

Any reason for this duplication? Wasn't it better to have it in a separate <Annotations /> component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wasn't it better to have it in a separate component?

Having it as a separate component is the reason the chart disappears after the annotations is rendered. It is a workaround until this issue gets fixed elastic/elastic-charts#914

Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for fixing so quickly. I added a few comments, most notably that I think annotations should be in a separate context from the shared pointer event.

@@ -224,7 +224,7 @@ export function TransactionDistribution({
id="y-axis"
position={Position.Left}
ticks={3}
showGridLines
gridLine={{ visible: true }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

showGridLines is deprecated, changing all to use gridLine.

Copy link
Contributor

@formgeist formgeist left a comment

Choose a reason for hiding this comment

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

In general the disappearing charts issue seems have been resolved. I spotted a few differences in the placement of the chart itself within the panel, it seems to have shifted upwards. The annotations are now conflicting with the titles. There's additional space under the legend, which is why I imagine the chart container has moved upwards.

Screenshot 2020-11-25 at 12 44 25

Copy link
Member

@sorenlouv sorenlouv left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@cauemarcondes
Copy link
Contributor Author

In general the disappearing charts issue seems have been resolved. I spotted a few differences in the placement of the chart itself within the panel, it seems to have shifted upwards. The annotations are now conflicting with the titles. There's additional space under the legend, which is why I imagine the chart container has moved upwards.

@formgeist I'm trying to figure out with @markov00 what could be caused this problem. None of us changed anything that could cause this change. I'm going to move forward with this PR, and open a new open to fix this problem.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
apm 3.1MB 3.1MB +1.3KB

History

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

@cauemarcondes cauemarcondes merged commit 8804b50 into elastic:master Nov 25, 2020
@cauemarcondes cauemarcondes deleted the apm-empty-charts branch November 25, 2020 16:08
cauemarcondes added a commit to cauemarcondes/kibana that referenced this pull request Nov 25, 2020
* fixing charts

* addressing pr comments
cauemarcondes added a commit that referenced this pull request Nov 25, 2020
* fixing charts

* addressing pr comments
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 26, 2020
* master: (70 commits)
  [Uptime] Fix headers io-ts type (elastic#84089)
  [fleet] Add config options to accepted docker env vars (elastic#84338)
  [Fleet] Support URL query state in agent logs UI (elastic#84298)
  [basePathProxy] include query in redirect (elastic#84356)
  [Security Solution] Add Endpoint policy feature checks (elastic#83972)
  Fix issues with show_license_expiration (elastic#84361)
  [Security Solution][Resolver] Add support for predefined schemas for endpoint and winlogbeat (elastic#84103)
  [cli/dev] log a warning when --no-base-path is used with --dev (elastic#84354)
  [Fleet] Support input-level vars & templates (elastic#83878)
  [APM] Elastic chart issues (elastic#84238)
  [Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel (elastic#83873)
  redirect to visualize listing page when by value visualization editor doesn't have a value input (elastic#84287)
  add live region for field search (elastic#84310)
  [ML] Persisted URL state for Anomalies table (elastic#84314)
  [dev/cli] detect worker type using env, not cluster module (elastic#83977)
  [Workplace Search] Migrate DisplaySettings tree (elastic#84283)
  Deprecate `xpack.task_manager.index` setting (elastic#84155)
  [Search] Search batching using bfetch (again) (elastic#84043)
  Use .kibana instead of .kibana_current to mark migration completion (elastic#83373)
  [Monitoring] Only look at ES for the missing data alert for now (elastic#83839)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Nov 26, 2020
* master: (119 commits)
  [Uptime] Fix headers io-ts type (elastic#84089)
  [fleet] Add config options to accepted docker env vars (elastic#84338)
  [Fleet] Support URL query state in agent logs UI (elastic#84298)
  [basePathProxy] include query in redirect (elastic#84356)
  [Security Solution] Add Endpoint policy feature checks (elastic#83972)
  Fix issues with show_license_expiration (elastic#84361)
  [Security Solution][Resolver] Add support for predefined schemas for endpoint and winlogbeat (elastic#84103)
  [cli/dev] log a warning when --no-base-path is used with --dev (elastic#84354)
  [Fleet] Support input-level vars & templates (elastic#83878)
  [APM] Elastic chart issues (elastic#84238)
  [Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel (elastic#83873)
  redirect to visualize listing page when by value visualization editor doesn't have a value input (elastic#84287)
  add live region for field search (elastic#84310)
  [ML] Persisted URL state for Anomalies table (elastic#84314)
  [dev/cli] detect worker type using env, not cluster module (elastic#83977)
  [Workplace Search] Migrate DisplaySettings tree (elastic#84283)
  Deprecate `xpack.task_manager.index` setting (elastic#84155)
  [Search] Search batching using bfetch (again) (elastic#84043)
  Use .kibana instead of .kibana_current to mark migration completion (elastic#83373)
  [Monitoring] Only look at ES for the missing data alert for now (elastic#83839)
  ...
gmmorris added a commit to gmmorris/kibana that referenced this pull request Dec 9, 2020
* master: (119 commits)
  [Uptime] Fix headers io-ts type (elastic#84089)
  [fleet] Add config options to accepted docker env vars (elastic#84338)
  [Fleet] Support URL query state in agent logs UI (elastic#84298)
  [basePathProxy] include query in redirect (elastic#84356)
  [Security Solution] Add Endpoint policy feature checks (elastic#83972)
  Fix issues with show_license_expiration (elastic#84361)
  [Security Solution][Resolver] Add support for predefined schemas for endpoint and winlogbeat (elastic#84103)
  [cli/dev] log a warning when --no-base-path is used with --dev (elastic#84354)
  [Fleet] Support input-level vars & templates (elastic#83878)
  [APM] Elastic chart issues (elastic#84238)
  [Time to Visualize] Fix Unlink Action via Rollback of ReplacePanel (elastic#83873)
  redirect to visualize listing page when by value visualization editor doesn't have a value input (elastic#84287)
  add live region for field search (elastic#84310)
  [ML] Persisted URL state for Anomalies table (elastic#84314)
  [dev/cli] detect worker type using env, not cluster module (elastic#83977)
  [Workplace Search] Migrate DisplaySettings tree (elastic#84283)
  Deprecate `xpack.task_manager.index` setting (elastic#84155)
  [Search] Search batching using bfetch (again) (elastic#84043)
  Use .kibana instead of .kibana_current to mark migration completion (elastic#83373)
  [Monitoring] Only look at ES for the missing data alert for now (elastic#83839)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support v7.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants