PMM-14900 Add protection to RTA session against unsupported pmm-agent version#5116
PMM-14900 Add protection to RTA session against unsupported pmm-agent version#5116
Conversation
maxkondr
requested review from
JiriCtvrtka and
ademidoff
and removed request for
a team
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3 #5116 +/- ##
==========================================
- Coverage 46.25% 46.24% -0.01%
==========================================
Files 390 390
Lines 40097 40177 +80
==========================================
+ Hits 18546 18581 +35
+ Misses 19616 19607 -9
- Partials 1935 1989 +54
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
it returns the list of services that support RTA: by service type and pmm-agent version
|
|
||
| for _, svc := range serviceList { | ||
| // Check that service has pmm-agent with version supporting RTA. | ||
| pmmAgent, err := models.FindPMMAgentsForService(s.db.Querier, svc.ServiceID) |
There was a problem hiding this comment.
This line makes the ListServices endpoint very expensive - it will make O(n) calls to the database. Let's consider the following to make it faster:
- Fetch all agents
- Make an in-memory data structure that maps Agents to a ServiceId, i.e. of type map[string]*models.Agent
- Use the map instead of the call to the database
There was a problem hiding this comment.
the problem with the proposed approach is that Service is not directly linked to PMM Agent (doesn't have PMM Agent ID filled and visa versa).
In order to make such mapping it is required to repeat the logic from agent_helpers.FindPMMAgentsForService that results into the same -> no benefits.
There was a problem hiding this comment.
I checked the implementation of FindPMMAgentsForService, it's even worse - we are dealing with O(n*3) requests, which is way more expensive than it should be.
Let's discuss a better way.
* feat: fetch only available services for RTA * chore: improve typings * chore: avoid admin data leakage * chore: simplify typings * chore: remove orgRole from query key * chore: make mongodb key mandatory * chore: expect all types for RTA * fix: missing key on map * chore: disable sort by operation ID * fix: RealTimeSelection tests * fix: typings
fabio-silva
requested review from
fabio-silva and
matejkubinec
and removed request for
a team
|
@ademidoff I've opened a new pull request, #5136, to work on those changes. Once the pull request is ready, I'll request review from you. |
This reverts commit 7d162aa.
- Add results sorting for ListSessions and ListServices functions
- Add more comments to explain reasons - remove extra logic outside DB transactions
PMM-14900
New endpoint is introduced
/v1/realtimeanalytics/services:it has return interface equal to
/v1/inventory/servicesIn case user requested start RTA session for the MongoDB service that is registered in PMM via pmm-agent <3.7.0 version - API returns an error.
Link to the Feature Build: Percona-Lab/pmm-submodules#4256