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

[O11y][Redis] Migrate Key Metrics dashboard visualizations to lens in kibana 8.3.0 #5659

Conversation

harnish-elastic
Copy link
Contributor

@harnish-elastic harnish-elastic commented Mar 23, 2023

  • Enhancement

What does this PR do?

  • Migrate visualizations to lens for Redis integration package

  • Statistics for Redis Lens migration:

Redis dashboard Before Migration   After Migration  
  Lens Visualization Lens Visualization
[Metrics Redis] Keys 0 5 5 0

Checklist

  • I have added an entry to my package's changelog.yml file.
  • I have verified that panels are populated with data.
  • I have verified that panels are not distorted after being migrated to lens.
  • I have updated screenshots of dashboard.
  • I have verified that data count are matching and panel aggregations are same as before.

Author's checklist

  • Migrated panels should be removed from visualization folder.
  • Migrated visualizations are populating in Kibana version 8.3.0.

Related issues

Issues Identified

@elasticmachine
Copy link

elasticmachine commented Mar 23, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-04-11T09:05:09.813+0000

  • Duration: 13 min 53 sec

Test stats 🧪

Test Results
Failed 0
Passed 23
Skipped 0
Total 23

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link

elasticmachine commented Mar 23, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 100.0% (1/1) 💚 50.0
Classes 100.0% (1/1) 💚 50.0
Methods 66.667% (14/21) 👎 -3.333
Lines 94.186% (81/86) 👍 23.952
Conditionals 100.0% (0/0) 💚

@kush-elastic kush-elastic mentioned this pull request Mar 23, 2023
7 tasks
@harnish-elastic harnish-elastic marked this pull request as ready for review March 23, 2023 12:39
@harnish-elastic harnish-elastic requested a review from a team as a code owner March 23, 2023 12:39
@kush-elastic kush-elastic added enhancement New feature or request Integration:redis Redis v8.3.0 Team:Service-Integrations Label for the Service Integrations team labels Mar 23, 2023
@harnish-elastic
Copy link
Contributor Author

Since we have confirmation on work-around of split series charts by Subhrata, putting this PR into draft.

@harnish-elastic harnish-elastic marked this pull request as draft March 24, 2023 05:49
@rajvi-patel-22
Copy link
Contributor

@SubhrataK , The two panels that were blocked in the Redis integration can be migrated as per the discussion that we had on split series functionality.

In Lists length [Metrics Redis] visualization, we found that the fields used in both split series functionality use different sizes (i.e. 16, 20 respectively) as highlighted in below screenshot. So it will show total 16*20=320 line charts (Top 16 values of redis.keys.id and Top 20 values of redis.key.name for each 16 redis.keys.id)!

image

However this is not the case in Lens. We use breakdown functionality in Lens to tackle this issue but in that we can specify only one size for both fields. For example if we provide 20 then it will show top 20 line chart for combined field of redis.keys.id + redis.keys.name. So while migrating this panel in lens, should we provide 320 (which will show 320 line charts and IMO it would become tough for the user to interpret the information) or should we provide the small number (for example 20 which will show user just top 20 charts)

image

And should we enable group other functionality in the lens visualization? it will group all the other values in separate bucket and provide one line chart for that.

@SubhrataK
Copy link

@rajvi-elastic As discussed, please change it to display Top 10 values only.

…into lens_migration-redis-key_metric-8.3.0

Conflicts:
	packages/redis/changelog.yml
	packages/redis/manifest.yml
@harnish-elastic harnish-elastic marked this pull request as ready for review April 6, 2023 11:26
@harnish-elastic
Copy link
Contributor Author

@agithomas Updated the PR with suggested changes. Thanks!

@agithomas
Copy link
Contributor

@SubhrataK , can we have your input on the discussion?

…into lens_migration-redis-key_metric-8.3.0

Conflicts:
	packages/redis/changelog.yml
	packages/redis/manifest.yml
@harnish-elastic
Copy link
Contributor Author

@SubhrataK , can we have your input on the discussion?

@agithomas For this discussion, @SubhrataK has provided her comments over here. But @SubhrataK can provide more information around it if it's needed.

Copy link
Contributor

@agithomas agithomas left a comment

Choose a reason for hiding this comment

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

LGTM!

…into lens_migration-redis-key_metric-8.3.0

Conflicts:
	packages/redis/changelog.yml
@kush-elastic kush-elastic merged commit ab3d0a1 into elastic:main Apr 11, 2023
@elasticmachine
Copy link

Package redis - 1.8.0 containing this change is available at https://epr.elastic.co/search?package=redis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Integration:redis Redis Team:Service-Integrations Label for the Service Integrations team v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants