mirror of
https://github.com/prometheus/prometheus.git
synced 2026-05-05 04:16:15 +02:00
feat: add destroy() method to PromQLExtension for memory leak prevention
When React components mount/unmount repeatedly, each creating a new PromQLExtension, memory leaks occur due to LRU caches with ttlAutopurge timers keeping references alive and in-flight HTTP requests holding closure references. This adds destroy() methods throughout the class hierarchy to properly release resources on unmount. Changes: - Add destroy() to PrometheusClient interface (optional) - HTTPPrometheusClient: track AbortControllers and abort pending requests - Cache: clear all LRU caches and reset cached data - CachedPrometheusClient: delegate to cache and underlying client - HybridComplete: delegate to prometheusClient - CompleteStrategy: add optional destroy() method - PromQLExtension: delegate to complete strategy Signed-off-by: Ben Blackmore <ben.blackmore@dash0.com>
This commit is contained in:
parent
041228bfcd
commit
31f046f416
@ -0,0 +1,97 @@
|
||||
// Copyright 2025 The Prometheus Authors
|
||||
// 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.
|
||||
|
||||
import { HTTPPrometheusClient, CachedPrometheusClient } from './prometheus';
|
||||
|
||||
describe('HTTPPrometheusClient destroy', () => {
|
||||
it('should be safe to call destroy multiple times', () => {
|
||||
const client = new HTTPPrometheusClient({ url: 'http://localhost:8080' });
|
||||
// First call
|
||||
client.destroy();
|
||||
// Second call should not throw
|
||||
expect(() => client.destroy()).not.toThrow();
|
||||
});
|
||||
|
||||
it('should abort in-flight requests when destroy is called', async () => {
|
||||
let abortSignal: AbortSignal | null | undefined;
|
||||
|
||||
const mockFetch = (_url: RequestInfo, init?: RequestInit): Promise<Response> => {
|
||||
abortSignal = init?.signal;
|
||||
// Return a promise that never resolves to simulate an in-flight request
|
||||
return new Promise(() => {});
|
||||
};
|
||||
|
||||
const client = new HTTPPrometheusClient({
|
||||
url: 'http://localhost:8080',
|
||||
fetchFn: mockFetch,
|
||||
});
|
||||
|
||||
// Start a request (don't await it)
|
||||
client.labelNames();
|
||||
|
||||
// Verify the signal was captured and not aborted yet
|
||||
expect(abortSignal).toBeDefined();
|
||||
expect(abortSignal?.aborted).toBe(false);
|
||||
|
||||
// Destroy the client
|
||||
client.destroy();
|
||||
|
||||
// Verify the request was aborted
|
||||
expect(abortSignal?.aborted).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('CachedPrometheusClient destroy', () => {
|
||||
it('should be safe to call destroy multiple times', () => {
|
||||
const httpClient = new HTTPPrometheusClient({ url: 'http://localhost:8080' });
|
||||
const cachedClient = new CachedPrometheusClient(httpClient);
|
||||
|
||||
// First call
|
||||
cachedClient.destroy();
|
||||
// Second call should not throw
|
||||
expect(() => cachedClient.destroy()).not.toThrow();
|
||||
});
|
||||
|
||||
it('should call destroy on the underlying HTTPPrometheusClient', () => {
|
||||
const httpClient = new HTTPPrometheusClient({ url: 'http://localhost:8080' });
|
||||
|
||||
let destroyCalled = false;
|
||||
const originalDestroy = httpClient.destroy.bind(httpClient);
|
||||
httpClient.destroy = () => {
|
||||
destroyCalled = true;
|
||||
originalDestroy();
|
||||
};
|
||||
|
||||
const cachedClient = new CachedPrometheusClient(httpClient);
|
||||
cachedClient.destroy();
|
||||
|
||||
expect(destroyCalled).toBe(true);
|
||||
});
|
||||
|
||||
it('should handle underlying clients without destroy method', () => {
|
||||
// Create a minimal PrometheusClient without destroy
|
||||
const minimalClient = {
|
||||
labelNames: () => Promise.resolve([]),
|
||||
labelValues: () => Promise.resolve([]),
|
||||
metricMetadata: () => Promise.resolve({}),
|
||||
series: () => Promise.resolve([]),
|
||||
metricNames: () => Promise.resolve([]),
|
||||
flags: () => Promise.resolve({}),
|
||||
};
|
||||
|
||||
const cachedClient = new CachedPrometheusClient(minimalClient);
|
||||
|
||||
// Should not throw even though underlying client has no destroy
|
||||
expect(() => cachedClient.destroy()).not.toThrow();
|
||||
});
|
||||
});
|
||||
@ -39,6 +39,9 @@ export interface PrometheusClient {
|
||||
|
||||
// flags returns flag values that prometheus was configured with.
|
||||
flags(): Promise<Record<string, string>>;
|
||||
|
||||
// destroy is called to release all resources held by this client
|
||||
destroy?(): void;
|
||||
}
|
||||
|
||||
export interface CacheConfig {
|
||||
@ -88,6 +91,7 @@ export class HTTPPrometheusClient implements PrometheusClient {
|
||||
// when calling it, thus the indirection via another function wrapper.
|
||||
private readonly fetchFn: FetchFn = (input: RequestInfo, init?: RequestInit): Promise<Response> => fetch(input, init);
|
||||
private requestHeaders: Headers = new Headers();
|
||||
private readonly abortControllers: Set<AbortController> = new Set<AbortController>();
|
||||
|
||||
constructor(config: PrometheusConfig) {
|
||||
this.url = config.url ? config.url : '';
|
||||
@ -199,11 +203,22 @@ export class HTTPPrometheusClient implements PrometheusClient {
|
||||
});
|
||||
}
|
||||
|
||||
destroy(): void {
|
||||
for (const controller of this.abortControllers) {
|
||||
controller.abort();
|
||||
}
|
||||
this.abortControllers.clear();
|
||||
}
|
||||
|
||||
private fetchAPI<T>(resource: string, init?: RequestInit): Promise<T> {
|
||||
const controller = new AbortController();
|
||||
this.abortControllers.add(controller);
|
||||
|
||||
if (init) {
|
||||
init.headers = this.requestHeaders;
|
||||
init.signal = controller.signal;
|
||||
} else {
|
||||
init = { headers: this.requestHeaders };
|
||||
init = { headers: this.requestHeaders, signal: controller.signal };
|
||||
}
|
||||
return this.fetchFn(this.url + resource, init)
|
||||
.then((res) => {
|
||||
@ -221,6 +236,9 @@ export class HTTPPrometheusClient implements PrometheusClient {
|
||||
throw new Error('missing "data" field in response JSON');
|
||||
}
|
||||
return apiRes.data;
|
||||
})
|
||||
.finally(() => {
|
||||
this.abortControllers.delete(controller);
|
||||
});
|
||||
}
|
||||
|
||||
@ -448,4 +466,8 @@ export class CachedPrometheusClient implements PrometheusClient {
|
||||
return flags;
|
||||
});
|
||||
}
|
||||
|
||||
destroy(): void {
|
||||
this.client.destroy?.();
|
||||
}
|
||||
}
|
||||
|
||||
@ -575,6 +575,10 @@ export class HybridComplete implements CompleteStrategy {
|
||||
return this.prometheusClient;
|
||||
}
|
||||
|
||||
destroy(): void {
|
||||
this.prometheusClient?.destroy?.();
|
||||
}
|
||||
|
||||
promQL(context: CompletionContext): Promise<CompletionResult | null> | CompletionResult | null {
|
||||
const { state, pos } = context;
|
||||
const tree = syntaxTree(state).resolve(pos, -1);
|
||||
|
||||
@ -19,6 +19,7 @@ import { CompletionContext, CompletionResult } from '@codemirror/autocomplete';
|
||||
// Every different completion mode must implement this interface.
|
||||
export interface CompleteStrategy {
|
||||
promQL(context: CompletionContext): Promise<CompletionResult | null> | CompletionResult | null;
|
||||
destroy?(): void;
|
||||
}
|
||||
|
||||
// CompleteConfiguration should be used to customize the autocompletion.
|
||||
|
||||
58
web/ui/module/codemirror-promql/src/promql.test.ts
Normal file
58
web/ui/module/codemirror-promql/src/promql.test.ts
Normal file
@ -0,0 +1,58 @@
|
||||
// Copyright 2025 The Prometheus Authors
|
||||
// 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.
|
||||
|
||||
import { PromQLExtension } from './promql';
|
||||
import { CompleteStrategy } from './complete';
|
||||
import { CompletionResult } from '@codemirror/autocomplete';
|
||||
|
||||
describe('PromQLExtension destroy', () => {
|
||||
it('should be safe to call destroy multiple times', () => {
|
||||
const extension = new PromQLExtension();
|
||||
// First call
|
||||
extension.destroy();
|
||||
// Second call should not throw
|
||||
expect(() => extension.destroy()).not.toThrow();
|
||||
});
|
||||
|
||||
it('should call destroy on the complete strategy if available', () => {
|
||||
const extension = new PromQLExtension();
|
||||
|
||||
// Set up a mock complete strategy with destroy
|
||||
let destroyCalled = false;
|
||||
const mockCompleteStrategy: CompleteStrategy = {
|
||||
promQL: (): CompletionResult | null => null,
|
||||
destroy: () => {
|
||||
destroyCalled = true;
|
||||
},
|
||||
};
|
||||
|
||||
extension.setComplete({ completeStrategy: mockCompleteStrategy });
|
||||
extension.destroy();
|
||||
|
||||
expect(destroyCalled).toBe(true);
|
||||
});
|
||||
|
||||
it('should handle complete strategies without destroy method', () => {
|
||||
const extension = new PromQLExtension();
|
||||
|
||||
// Set up a mock complete strategy without destroy
|
||||
const mockCompleteStrategy: CompleteStrategy = {
|
||||
promQL: (): CompletionResult | null => null,
|
||||
};
|
||||
|
||||
extension.setComplete({ completeStrategy: mockCompleteStrategy });
|
||||
|
||||
// Should not throw even though complete strategy has no destroy
|
||||
expect(() => extension.destroy()).not.toThrow();
|
||||
});
|
||||
});
|
||||
@ -79,6 +79,10 @@ export class PromQLExtension {
|
||||
return this;
|
||||
}
|
||||
|
||||
destroy(): void {
|
||||
this.complete.destroy?.();
|
||||
}
|
||||
|
||||
asExtension(languageType = LanguageType.PromQL): Extension {
|
||||
const language = promQLLanguage(languageType);
|
||||
let extension: Extension = [language];
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user