-
-
Notifications
You must be signed in to change notification settings - Fork 727
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
feat: store memory footprints to grafana #9001
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,6 +35,7 @@ import { | |
import type ConfigurationRevisionService from '../feature-toggle/configuration-revision-service'; | ||
import type { ClientFeatureToggleService } from './client-feature-toggle-service'; | ||
import { | ||
CLIENT_FEATURES_MEMORY, | ||
CLIENT_METRICS_NAMEPREFIX, | ||
CLIENT_METRICS_TAGS, | ||
} from '../../internals'; | ||
|
@@ -69,6 +70,8 @@ export default class FeatureController extends Controller { | |
|
||
private eventBus: EventEmitter; | ||
|
||
private clientFeaturesCacheMap = new Map<string, number>(); | ||
|
||
private featuresAndSegments: ( | ||
query: IFeatureToggleQuery, | ||
etag: string, | ||
|
@@ -162,6 +165,32 @@ export default class FeatureController extends Controller { | |
private async resolveFeaturesAndSegments( | ||
query?: IFeatureToggleQuery, | ||
): Promise<[FeatureConfigurationClient[], IClientSegment[]]> { | ||
if (this.flagResolver.isEnabled('deltaApi')) { | ||
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. Only if flag on, do this logic, otherwise go default. We will start very gradual rollout. |
||
const features = | ||
await this.clientFeatureToggleService.getClientFeatures(query); | ||
|
||
const segments = | ||
await this.clientFeatureToggleService.getActiveSegmentsForClient(); | ||
|
||
try { | ||
const featuresSize = this.getCacheSizeInBytes(features); | ||
const segmentsSize = this.getCacheSizeInBytes(segments); | ||
this.clientFeaturesCacheMap.set( | ||
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. Saving the current key into cache. 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. Again: can query be undefined here (because we force it with So is this the size of the query without delta or with? 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. Ah, I see now. This is specified in the store footprint method. This is for features. |
||
JSON.stringify(query), | ||
featuresSize + segmentsSize, | ||
); | ||
|
||
await this.clientFeatureToggleService.getClientDelta( | ||
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. We are just making the same call in delta API to request for cache update. |
||
undefined, | ||
query!, | ||
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. When will query not be present? This |
||
); | ||
this.storeFootprint(); | ||
} catch (e) { | ||
this.logger.error('Delta diff failed', e); | ||
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. Putting all in try catch if something fails for extra safety. |
||
} | ||
|
||
return [features, segments]; | ||
} | ||
return Promise.all([ | ||
this.clientFeatureToggleService.getClientFeatures(query), | ||
this.clientFeatureToggleService.getActiveSegmentsForClient(), | ||
|
@@ -270,7 +299,6 @@ export default class FeatureController extends Controller { | |
query, | ||
etag, | ||
); | ||
|
||
if (this.clientSpecService.requestSupportsSpec(req, 'segments')) { | ||
this.openApiService.respondWithValidation( | ||
200, | ||
|
@@ -335,4 +363,17 @@ export default class FeatureController extends Controller { | |
}, | ||
); | ||
} | ||
|
||
storeFootprint() { | ||
let memory = 0; | ||
for (const value of this.clientFeaturesCacheMap.values()) { | ||
memory += value; | ||
} | ||
this.eventBus.emit(CLIENT_FEATURES_MEMORY, { memory }); | ||
} | ||
|
||
getCacheSizeInBytes(value: any): number { | ||
const jsonString = JSON.stringify(value); | ||
return Buffer.byteLength(jsonString, 'utf8'); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ import type { | |
IFeatureToggleQuery, | ||
IFlagResolver, | ||
ISegmentReadModel, | ||
IUnleashConfig, | ||
} from '../../../types'; | ||
import type ConfigurationRevisionService from '../../feature-toggle/configuration-revision-service'; | ||
import { UPDATE_REVISION } from '../../feature-toggle/configuration-revision-service'; | ||
|
@@ -13,6 +14,9 @@ import type { | |
FeatureConfigurationDeltaClient, | ||
IClientFeatureToggleDeltaReadModel, | ||
} from './client-feature-toggle-delta-read-model-type'; | ||
import { CLIENT_DELTA_MEMORY } from '../../../metric-events'; | ||
import type EventEmitter from 'events'; | ||
import type { Logger } from '../../../logger'; | ||
|
||
type DeletedFeature = { | ||
name: string; | ||
|
@@ -86,7 +90,6 @@ export const calculateRequiredClientRevision = ( | |
const targetedRevisions = revisions.filter( | ||
(revision) => revision.revisionId > requiredRevisionId, | ||
); | ||
console.log('targeted revisions', targetedRevisions); | ||
const projectFeatureRevisions = targetedRevisions.map((revision) => | ||
filterRevisionByProject(revision, projects), | ||
); | ||
|
@@ -105,27 +108,32 @@ export class ClientFeatureToggleDelta { | |
|
||
private currentRevisionId: number = 0; | ||
|
||
private interval: NodeJS.Timer; | ||
|
||
private flagResolver: IFlagResolver; | ||
|
||
private configurationRevisionService: ConfigurationRevisionService; | ||
|
||
private readonly segmentReadModel: ISegmentReadModel; | ||
|
||
private eventBus: EventEmitter; | ||
|
||
private readonly logger: Logger; | ||
|
||
constructor( | ||
clientFeatureToggleDeltaReadModel: IClientFeatureToggleDeltaReadModel, | ||
segmentReadModel: ISegmentReadModel, | ||
eventStore: IEventStore, | ||
configurationRevisionService: ConfigurationRevisionService, | ||
flagResolver: IFlagResolver, | ||
config: IUnleashConfig, | ||
) { | ||
this.eventStore = eventStore; | ||
this.configurationRevisionService = configurationRevisionService; | ||
this.clientFeatureToggleDeltaReadModel = | ||
clientFeatureToggleDeltaReadModel; | ||
this.flagResolver = flagResolver; | ||
this.segmentReadModel = segmentReadModel; | ||
this.eventBus = config.eventBus; | ||
this.logger = config.getLogger('delta/client-feature-toggle-delta.js'); | ||
this.onUpdateRevisionEvent = this.onUpdateRevisionEvent.bind(this); | ||
this.delta = {}; | ||
|
||
|
@@ -161,6 +169,8 @@ export class ClientFeatureToggleDelta { | |
await this.updateSegments(); | ||
} | ||
|
||
// TODO: 19.12 this logic seems to be not logical, when no revisionId is coming, it should not go to db, but take latest from cache | ||
|
||
// Should get the latest state if revision does not exist or if sdkRevision is not present | ||
// We should be able to do this without going to the database by merging revisions from the delta with | ||
// the base case | ||
|
@@ -203,12 +213,13 @@ export class ClientFeatureToggleDelta { | |
|
||
private async onUpdateRevisionEvent() { | ||
if (this.flagResolver.isEnabled('deltaApi')) { | ||
await this.listenToRevisionChange(); | ||
await this.updateFeaturesDelta(); | ||
await this.updateSegments(); | ||
this.storeFootprint(); | ||
} | ||
} | ||
|
||
public async listenToRevisionChange() { | ||
public async updateFeaturesDelta() { | ||
const keys = Object.keys(this.delta); | ||
|
||
if (keys.length === 0) return; | ||
|
@@ -248,7 +259,6 @@ export class ClientFeatureToggleDelta { | |
removed, | ||
}); | ||
} | ||
|
||
this.currentRevisionId = latestRevision; | ||
} | ||
|
||
|
@@ -279,8 +289,9 @@ export class ClientFeatureToggleDelta { | |
removed: [], | ||
}, | ||
]); | ||
|
||
this.delta[environment] = delta; | ||
|
||
this.storeFootprint(); | ||
} | ||
|
||
async getClientFeatures( | ||
|
@@ -294,4 +305,20 @@ export class ClientFeatureToggleDelta { | |
private async updateSegments(): Promise<void> { | ||
this.segments = await this.segmentReadModel.getActiveForClient(); | ||
} | ||
|
||
storeFootprint() { | ||
try { | ||
const featuresMemory = this.getCacheSizeInBytes(this.delta); | ||
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. How many of these do we keep at the same time? Help me wrap my head around this: can the delta be different for each connected client (assuming they're polling and you're making a flag change every second)? presumably this uses the revision id to store something, right, so it doesn't need that. I ... remember you saying something about that. 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. We keep a delta for each environment. So all clients that connect from same environment, share it. |
||
const segmentsMemory = this.getCacheSizeInBytes(this.segments); | ||
const memory = featuresMemory + segmentsMemory; | ||
this.eventBus.emit(CLIENT_DELTA_MEMORY, { memory }); | ||
} catch (e) { | ||
this.logger.error('Client delta footprint error', e); | ||
} | ||
} | ||
|
||
getCacheSizeInBytes(value: any): number { | ||
const jsonString = JSON.stringify(value); | ||
return Buffer.byteLength(jsonString, 'utf8'); | ||
} | ||
} |
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.
We can not read the size of memoizee directly, so we will be storing the memory usage for each query key into new cache. This will be calculated on new revision id.