-
Notifications
You must be signed in to change notification settings - Fork 1.9k
add OpenTelemetry metrics instrumentation #3110
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
base: master
Are you sure you want to change the base?
Changes from 43 commits
2d569f7
364ed40
2e3cb2a
cd31dfa
0d55b20
67d99ca
ce74dc1
0eedb01
a14d159
46ebd09
cfe88dc
bccbdf7
91305c4
28c1a1b
27a090a
63d2bcf
3a0d9de
97e1539
d5bce49
eda93d2
bfa15be
c2bc36e
55aba1d
f0a47cb
9a08be7
41f0188
d22a0fb
996d4ee
c0feedb
406a9c6
d167cc9
d377b92
ce23121
89aaf1b
912e27d
ad2fd8d
e843d0d
6d14d9c
4305d30
19c95b8
34cd991
ee5f2b0
d0f42d3
3e19cad
baac384
08c7413
f8a51c9
181d086
bc14732
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,7 @@ import { EventEmitter } from 'stream'; | |
| import RedisClient from '.'; | ||
| import { RedisArgument, ReplyUnion, TransformReply, TypeMapping } from '../RESP/types'; | ||
| import { BasicCommandParser } from './parser'; | ||
| import { OTelMetrics, CSC_RESULT, CSC_EVICTION_REASON } from '../opentelemetry'; | ||
|
|
||
| /** | ||
| * A snapshot of cache statistics. | ||
|
|
@@ -484,6 +485,7 @@ export abstract class ClientSideCacheProvider extends EventEmitter { | |
| abstract invalidate(key: RedisArgument | null): void; | ||
| abstract clear(): void; | ||
| abstract stats(): CacheStats; | ||
| abstract size(): number; | ||
| abstract onError(): void; | ||
| abstract onClose(): void; | ||
| } | ||
|
|
@@ -551,21 +553,41 @@ export class BasicClientSideCache extends ClientSideCacheProvider { | |
|
|
||
| // "2" | ||
| let cacheEntry = this.get(cacheKey); | ||
|
|
||
| if (cacheEntry) { | ||
| // If instanceof is "too slow", can add a "type" and then use an "as" cast to call proper getters. | ||
| if (cacheEntry instanceof ClientSideCacheEntryValue) { // "2b1" | ||
| this.#statsCounter.recordHits(1); | ||
| OTelMetrics.instance.clientSideCacheMetrics.recordCacheRequest( | ||
| CSC_RESULT.HIT, | ||
| client._clientId, | ||
vladvildanov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| ); | ||
| // Estimate bytes saved by avoiding network round-trip | ||
| // Note: JSON.stringify approximation; actual RESP wire size may differ (especially for Buffers) | ||
| const bytesEstimate = JSON.stringify(cacheEntry.value).length; | ||
|
||
| OTelMetrics.instance.clientSideCacheMetrics.recordNetworkBytesSaved( | ||
| bytesEstimate, | ||
| client._clientId, | ||
| ); | ||
|
|
||
| return structuredClone(cacheEntry.value); | ||
| } else if (cacheEntry instanceof ClientSideCacheEntryPromise) { // 2b2 | ||
| // This counts as a miss since the value hasn't been fully loaded yet. | ||
| this.#statsCounter.recordMisses(1); | ||
| OTelMetrics.instance.clientSideCacheMetrics.recordCacheRequest( | ||
vladvildanov marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| CSC_RESULT.MISS, | ||
| client._clientId, | ||
| ); | ||
| reply = await cacheEntry.promise; | ||
| } else { | ||
| throw new Error("unknown cache entry type"); | ||
| } | ||
| } else { // 3/3a | ||
| this.#statsCounter.recordMisses(1); | ||
| OTelMetrics.instance.clientSideCacheMetrics.recordCacheRequest( | ||
| CSC_RESULT.MISS, | ||
| client._clientId, | ||
| ); | ||
|
|
||
| const startTime = performance.now(); | ||
| const promise = fn(); | ||
|
|
@@ -616,22 +638,34 @@ export class BasicClientSideCache extends ClientSideCacheProvider { | |
|
|
||
| override invalidate(key: RedisArgument | null) { | ||
| if (key === null) { | ||
| // Server requested to invalidate all keys | ||
| const oldSize = this.size(); | ||
| this.clear(false); | ||
| // Record invalidations as server-initiated evictions | ||
| if (oldSize > 0) { | ||
| OTelMetrics.instance.clientSideCacheMetrics.recordCacheEviction(CSC_EVICTION_REASON.INVALIDATION, oldSize); | ||
|
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. Cache eviction metrics missing
|
||
| } | ||
| this.emit("invalidate", key); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| const keySet = this.#keyToCacheKeySetMap.get(key.toString()); | ||
| if (keySet) { | ||
| let deletedCount = 0; | ||
| for (const cacheKey of keySet) { | ||
| const entry = this.#cacheKeyToEntryMap.get(cacheKey); | ||
| if (entry) { | ||
| entry.invalidate(); | ||
| deletedCount++; | ||
| } | ||
| this.#cacheKeyToEntryMap.delete(cacheKey); | ||
| } | ||
| this.#keyToCacheKeySetMap.delete(key.toString()); | ||
| if (deletedCount > 0) { | ||
| // Record invalidations as server-initiated evictions | ||
| OTelMetrics.instance.clientSideCacheMetrics.recordCacheEviction(CSC_EVICTION_REASON.INVALIDATION, deletedCount); | ||
| } | ||
| } | ||
|
|
||
| this.emit('invalidate', key); | ||
|
|
@@ -660,6 +694,8 @@ export class BasicClientSideCache extends ClientSideCacheProvider { | |
| if (val && !val.validate()) { | ||
| this.delete(cacheKey); | ||
| this.#statsCounter.recordEvictions(1); | ||
| // Entry failed validation - this is TTL expiry since invalidation marks are handled separately | ||
| OTelMetrics.instance.clientSideCacheMetrics.recordCacheEviction(CSC_EVICTION_REASON.TTL); | ||
| this.emit("cache-evict", cacheKey); | ||
|
|
||
| return undefined; | ||
|
|
@@ -690,13 +726,15 @@ export class BasicClientSideCache extends ClientSideCacheProvider { | |
| const oldEntry = this.#cacheKeyToEntryMap.get(cacheKey); | ||
|
|
||
| if (oldEntry) { | ||
| count--; // overwriting, so not incrementig | ||
| count--; // overwriting, so not incrementing | ||
| oldEntry.invalidate(); | ||
| } | ||
|
|
||
| if (this.maxEntries > 0 && count >= this.maxEntries) { | ||
| this.deleteOldest(); | ||
| this.#statsCounter.recordEvictions(1); | ||
| // Eviction due to cache capacity limit | ||
| OTelMetrics.instance.clientSideCacheMetrics.recordCacheEviction(CSC_EVICTION_REASON.FULL); | ||
| } | ||
|
|
||
| this.#cacheKeyToEntryMap.set(cacheKey, cacheEntry); | ||
|
|
||


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.
@nkaradzhov we might want to rename
onSuccessto something more metrics/otel related