Skip to content

PMM-13576 valkey support #4052

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

Draft
wants to merge 32 commits into
base: v3
Choose a base branch
from
Draft

PMM-13576 valkey support #4052

wants to merge 32 commits into from

Conversation

idoqo
Copy link
Contributor

@idoqo idoqo commented May 27, 2025

PMM-13576

Link to the Feature Build: SUBMODULES-3945

If this PR adds or removes or alters one or more API endpoints, please review and add or update the relevant API documents as well:

  • API Docs updated

If this PR is related to some other PRs in this or other repositories, please provide links to those PRs:

  • Links to related pull requests (optional).

talhabinrizwan and others added 12 commits May 21, 2025 14:50
* PMM-13838 Add Valkey to PMM data model

* PMM-13838 Remove rendundant params

* PMM-13838 Add Valkey to agent and service servers

* PMM-13838 Follow up on reported linter errors

* PMM-13838 Follow up on reported linter errors

* Update api/inventory/v1/agents.proto

Co-authored-by: Michael Okoko <[email protected]>

* PMM-13838 Follow up on PR review

* PMM-13838 Revert to errors.New

* PMM-13838 Update the linter rules

* implement api methods to add valkey service

* filter service type

* add valkey service

* reformat and set up inventory agents API

* set up change agent params endpoint

* add copyright header

* update license copyright year

* include expose exporter option

* surpress lint errors

* set up connection checker for valkey

* set up service info broker for valkey

* fix tests

* add TLS support for valkey connection checks

* fix linter errors

* fix skip verify parameter

* ignore gosec lint

* align params to satisfy linter

* add api tests

* drop unused parameter

* add option to use redis scheme in URL

* fix tests

* document data model

* fix typos

* use tls param for tls check

* fix linters and tests

* drop unused field

* trigger rebuild

* fix api tests

* drop unnecessary toLower func

---------

Co-authored-by: Alex Demidoff <[email protected]>
Copy link

codecov bot commented May 27, 2025

Codecov Report

Attention: Patch coverage is 30.29851% with 467 lines in your changes missing coverage. Please review.

Project coverage is 44.00%. Comparing base (854e2b2) to head (c676c92).

Files with missing lines Patch % Lines
admin/commands/management/add_valkey.go 2.35% 83 Missing ⚠️
managed/services/management/valkey.go 0.00% 71 Missing ⚠️
...in/commands/inventory/add_agent_valkey_exporter.go 0.00% 53 Missing ⚠️
managed/services/inventory/agents.go 45.71% 32 Missing and 6 partials ⚠️
admin/commands/inventory/add_service_valkey.go 6.89% 27 Missing ⚠️
managed/services/agents/valkey.go 0.00% 27 Missing ⚠️
managed/services/inventory/grpc/services_server.go 0.00% 26 Missing ⚠️
agent/tlshelpers/valkey.go 24.13% 21 Missing and 1 partial ⚠️
managed/models/agent_model.go 50.00% 20 Missing and 1 partial ⚠️
agent/serviceinfobroker/service_info_broker.go 69.64% 13 Missing and 4 partials ⚠️
... and 13 more
Additional details and impacted files
@@            Coverage Diff             @@
##               v3    #4052      +/-   ##
==========================================
- Coverage   44.15%   44.00%   -0.16%     
==========================================
  Files         369      375       +6     
  Lines       45306    45957     +651     
==========================================
+ Hits        20007    20224     +217     
- Misses      23612    24030     +418     
- Partials     1687     1703      +16     
Flag Coverage Δ
admin 17.13% <14.81%> (-0.11%) ⬇️
agent 52.66% <57.48%> (-0.08%) ⬇️
managed 44.92% <29.96%> (-0.05%) ⬇️
vmproxy 73.45% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

talhabinrizwan and others added 6 commits May 28, 2025 09:59
* PMM-13838 Add Valkey to PMM data model

* PMM-13838 Remove rendundant params

* PMM-13838 Add Valkey to agent and service servers

* PMM-13838 Follow up on reported linter errors

* PMM-13838 Follow up on reported linter errors

* Update api/inventory/v1/agents.proto

Co-authored-by: Michael Okoko <[email protected]>

* PMM-13838 Follow up on PR review

* PMM-13838 Revert to errors.New

* PMM-13838 Update the linter rules

* implement api methods to add valkey service

* filter service type

* add valkey service

* reformat and set up inventory agents API

* set up change agent params endpoint

* add copyright header

* update license copyright year

* include expose exporter option

* surpress lint errors

* set up connection checker for valkey

* set up service info broker for valkey

* fix tests

* add TLS support for valkey connection checks

* fix linter errors

* fix skip verify parameter

* ignore gosec lint

* align params to satisfy linter

* Initial pmm-agent execution support for valkey

This includes triggering the binary update from supervisord and passing
the relevant args/flags to the valkey_exporter.

One more big part left is passing in the correct flags to support TLS,
and checking the exporter docs for other flags that we might need to
use.

* fix valkey status

* fix self-signed certs usage

* fix linter errors

* add api tests

* drop unused parameter

* add option to use redis scheme in URL

* fix tests

* document data model

* fix typos

* drop unused redis scheme

---------

Co-authored-by: Alex Demidoff <[email protected]>
@ademidoff ademidoff force-pushed the PMM-13576-valkey-support branch from 87b4ae5 to 6188368 Compare May 31, 2025 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants