-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Introduce inventory service counts. #25755
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
Changes from all commits
3b6b583
3391cb5
0205db8
0dcfeab
39a7bd1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| /* | ||
| Copyright 2023 Gravitational, Inc. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package inventory | ||
|
|
||
| import ( | ||
| "sync" | ||
| "sync/atomic" | ||
|
|
||
| "github.com/gravitational/teleport/api/types" | ||
| ) | ||
|
|
||
| // serviceCounter will count services seen in the inventory. | ||
| type serviceCounter struct { | ||
| countMap sync.Map | ||
| } | ||
|
|
||
| // counts returns the count of each service seen in the counter. | ||
| func (s *serviceCounter) counts() map[types.SystemRole]uint64 { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it worth it to add a dedicated function to fetch the count for a single role?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was waffling about that. It makes sense, but it's also not a huge deal to just query the map. I'm ambivalent, I think. If you feel strongly I can add one.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I just realized that I wouldn't bother with wiring it up for the gRPC service (making a map is nothing compared to the overhead of protobuf encoding and network transmission, and saving like 5 strings and integers worth of data is kinda useless) but it might be useful as an exposed |
||
| counts := map[types.SystemRole]uint64{} | ||
| s.countMap.Range(func(key, value any) bool { | ||
| counts[key.(types.SystemRole)] = value.(*atomic.Uint64).Load() | ||
| return true | ||
| }) | ||
|
|
||
| return counts | ||
| } | ||
|
|
||
| // increment will increment the counter for a service. | ||
| func (s *serviceCounter) increment(service types.SystemRole) { | ||
| s.load(service).Add(1) | ||
| } | ||
|
|
||
| // decrement will decrement the counter for a service. | ||
| func (s *serviceCounter) decrement(service types.SystemRole) { | ||
| // refer to the docs for atomic.AddUint64 for why this works as a decrement. | ||
| s.load(service).Add(^uint64(0)) | ||
| } | ||
|
|
||
| // get will return the value of a counter for a service. | ||
| func (s *serviceCounter) get(service types.SystemRole) uint64 { | ||
| if result, ok := s.countMap.Load(service); ok { | ||
| return result.(*atomic.Uint64).Load() | ||
| } | ||
| return 0 | ||
| } | ||
|
|
||
| // load will load the underlying atomic value in the sync map. This should | ||
| // only be used within the service counter. | ||
| func (s *serviceCounter) load(service types.SystemRole) *atomic.Uint64 { | ||
| if result, ok := s.countMap.Load(service); ok { | ||
| return result.(*atomic.Uint64) | ||
| } | ||
| result, _ := s.countMap.LoadOrStore(service, &atomic.Uint64{}) | ||
|
mdwn marked this conversation as resolved.
|
||
| return result.(*atomic.Uint64) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| /* | ||
| Copyright 2023 Gravitational, Inc. | ||
|
|
||
| Licensed under the Apache License, Version 2.0 (the "License"); | ||
| you may not use this file except in compliance with the License. | ||
| You may obtain a copy of the License at | ||
|
|
||
| http://www.apache.org/licenses/LICENSE-2.0 | ||
|
|
||
| Unless required by applicable law or agreed to in writing, software | ||
| distributed under the License is distributed on an "AS IS" BASIS, | ||
| WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| See the License for the specific language governing permissions and | ||
| limitations under the License. | ||
| */ | ||
|
|
||
| package inventory | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/require" | ||
|
|
||
| "github.com/gravitational/teleport/api/types" | ||
| ) | ||
|
|
||
| func TestServiceCounter(t *testing.T) { | ||
| sc := serviceCounter{} | ||
|
|
||
| require.Equal(t, uint64(0), sc.get(types.RoleAuth)) | ||
|
|
||
| sc.increment(types.RoleApp) | ||
| require.Equal(t, uint64(1), sc.get(types.RoleApp)) | ||
| sc.increment(types.RoleApp) | ||
| require.Equal(t, uint64(2), sc.get(types.RoleApp)) | ||
|
|
||
| require.Equal(t, map[types.SystemRole]uint64{ | ||
| types.RoleApp: 2, | ||
| }, sc.counts()) | ||
|
|
||
| sc.decrement(types.RoleApp) | ||
| require.Equal(t, uint64(1), sc.get(types.RoleApp)) | ||
| sc.decrement(types.RoleApp) | ||
| require.Equal(t, uint64(0), sc.get(types.RoleApp)) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we also expose some sort of notification mechanism that triggers whenever some counts go from zero to non-zero and viceversa? I imagine it could be useful for other components in the auth server that depend on some agent to be connected but don't necessarily care about the exact number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Discussed a bit of this offline. This is useful but may not be immediately useful for the Okta service use case.