From 7ce9553152270be0234161f03149f7c60835d26f Mon Sep 17 00:00:00 2001 From: Gero Posmyk-Leinemann <32448529+geropl@users.noreply.github.com> Date: Tue, 10 Oct 2023 12:02:18 +0200 Subject: [PATCH] [server] SpiceDB: Add metric for consistency usage (#18901) * [server] More tests for ZedTokenCache * [server] Add metrics "incSpiceDBRequestsCheckTotal" --- .../caching-spicedb-authorizer.spec.db.ts | 74 ++++++++++++++++++- .../caching-spicedb-authorizer.ts | 5 +- components/server/src/prometheus-metrics.ts | 16 ++++ 3 files changed, 93 insertions(+), 2 deletions(-) diff --git a/components/server/src/authorization/caching-spicedb-authorizer.spec.db.ts b/components/server/src/authorization/caching-spicedb-authorizer.spec.db.ts index cf3072b716a575..6ef4d12614e5dc 100644 --- a/components/server/src/authorization/caching-spicedb-authorizer.spec.db.ts +++ b/components/server/src/authorization/caching-spicedb-authorizer.spec.db.ts @@ -15,10 +15,11 @@ import { Authorizer, SYSTEM_USER } from "./authorizer"; import { OrganizationService } from "../orgs/organization-service"; import { WorkspaceService } from "../workspace/workspace-service"; import { UserService } from "../user/user-service"; -import { ZedTokenCache } from "./caching-spicedb-authorizer"; +import { RequestLocalZedTokenCache, ZedTokenCache } from "./caching-spicedb-authorizer"; import { log } from "@gitpod/gitpod-protocol/lib/util/logging"; import { ConfigProvider } from "../workspace/config-provider"; import { runWithContext } from "../util/log-context"; +import { v1 } from "@authzed/authzed-node"; const expect = chai.expect; @@ -232,3 +233,74 @@ describe("CachingSpiceDBAuthorizer", async () => { ).to.be.true; }); }); + +describe("RequestLocalZedTokenCache", async () => { + let cache: RequestLocalZedTokenCache; + + const rawToken1 = "GhUKEzE2OTY0MjI3NzY1Njc3Mzc0MjQ="; + const rawToken2 = "GhUKEzE2OTY5Mjg1Nzg1NjIyNjYzMTE="; + const rawToken3 = "GhUKEzE2OTY5Mjg1Nzg1NjgwMTE3MzM="; + const ws1 = v1.ObjectReference.create({ + objectType: "workspace", + objectId: "ws1", + }); + + function fullyConsistent() { + return v1.Consistency.create({ + requirement: { + oneofKind: "fullyConsistent", + fullyConsistent: true, + }, + }); + } + + function atLeastAsFreshAs(zedToken: string) { + return v1.Consistency.create({ + requirement: { + oneofKind: "atLeastAsFresh", + atLeastAsFresh: v1.ZedToken.create({ + token: zedToken, + }), + }, + }); + } + + beforeEach(async () => { + cache = new RequestLocalZedTokenCache(); + }); + + it("should store token", async () => { + await runWithContext("test", {}, async () => { + expect(await cache.get(ws1)).to.be.undefined; + await cache.set([ws1, rawToken1]); + expect(await cache.get(ws1)).to.equal(rawToken1); + }); + }); + + it("should return newest token", async () => { + await runWithContext("test", {}, async () => { + await cache.set([ws1, rawToken1]); + await cache.set([ws1, rawToken2]); + expect(await cache.get(ws1)).to.equal(rawToken2); + await cache.set([ws1, rawToken3]); + expect(await cache.get(ws1)).to.equal(rawToken3); + }); + }); + + it("should return proper consistency", async () => { + await runWithContext("test", {}, async () => { + expect(await cache.consistency(ws1)).to.deep.equal(fullyConsistent()); + await cache.set([ws1, rawToken1]); + expect(await cache.consistency(ws1)).to.deep.equal(atLeastAsFreshAs(rawToken1)); + }); + }); + + it("should clear cache", async () => { + await runWithContext("test", {}, async () => { + await cache.set([ws1, rawToken1]); + expect(await cache.get(ws1)).to.equal(rawToken1); + await cache.set([ws1, undefined]); // this should trigger a clear + expect(await cache.get(ws1)).to.be.undefined; + }); + }); +}); diff --git a/components/server/src/authorization/caching-spicedb-authorizer.ts b/components/server/src/authorization/caching-spicedb-authorizer.ts index 4432c2d2e571e0..c43df104ac3240 100644 --- a/components/server/src/authorization/caching-spicedb-authorizer.ts +++ b/components/server/src/authorization/caching-spicedb-authorizer.ts @@ -11,6 +11,7 @@ import { inject, injectable } from "inversify"; import { clearZedTokenOnContext, getZedTokenFromContext, setZedTokenToContext } from "../util/log-context"; import { base64decode } from "@jmondi/oauth2-server"; import { DecodedZedToken } from "@gitpod/spicedb-impl/lib/impl/v1/impl.pb"; +import { incSpiceDBRequestsCheckTotal } from "../prometheus-metrics"; export type ZedTokenCacheKV = [objectRef: v1.ObjectReference | undefined, token: string | undefined]; export const ZedTokenCache = Symbol("ZedTokenCache"); @@ -36,6 +37,8 @@ export class CachingSpiceDBAuthorizer implements SpiceDBAuthorizer { forceEnablement?: boolean | undefined, ): Promise { req.consistency = await this.tokenCache.consistency(req.resource); + incSpiceDBRequestsCheckTotal(req.consistency?.requirement?.oneofKind || "undefined"); + const result = await this.impl.check(req, experimentsFields, forceEnablement); if (result.checkedAt) { await this.tokenCache.set([req.resource, result.checkedAt]); @@ -139,7 +142,7 @@ export class RequestLocalZedTokenCache implements ZedTokenCache { if (!prev || prev.timestamp < curr.timestamp) { return curr; } - return curr; + return prev; }, undefined); } } diff --git a/components/server/src/prometheus-metrics.ts b/components/server/src/prometheus-metrics.ts index 13485480781cb7..0d356dc0ae37fb 100644 --- a/components/server/src/prometheus-metrics.ts +++ b/components/server/src/prometheus-metrics.ts @@ -352,3 +352,19 @@ export type GuardAccessCheckType = "fga" | "resource-access"; export function reportGuardAccessCheck(type: GuardAccessCheckType) { guardAccessChecksTotal.labels(type).inc(); } + +export const spicedbCheckRequestsTotal = new prometheusClient.Counter({ + name: "gitpod_spicedb_requests_check_total", + help: "Counter for the number of check requests against SpiceDB", + labelNames: ["consistency"], +}); + +export type SpiceDBCheckConsistency = + | "minimizeLatency" + | "atLeastAsFresh" + | "atExactSnapshot" + | "fullyConsistent" + | "undefined"; +export function incSpiceDBRequestsCheckTotal(consistency: SpiceDBCheckConsistency) { + spicedbCheckRequestsTotal.labels(consistency).inc(); +}