Repository navigation
feat!: trace and measure scans, on forge v1.12.9 and grove v1.8.0 - #47
Merged
Merged
Conversation
Moves forge to v1.12.9 and grove plus its mongo, pg and sqlite drivers to v1.8.0. go-utils comes along to v1.4.1 because forge needs it. With this pin, grove records a db.client span and the operation duration for every store query, so you get database telemetry without any change in shield. Build, vet and the race tests pass unchanged.
Every ScanInput and ScanOutput call now runs under a "scan content" span from the github.com/xraph/shield tracer. It carries the scan ID, direction, decision, whether the scan blocked, the finding count and, when a store is configured, whether the result was persisted. A failed store write keeps the scan's verdict but marks the span persist_failed, so a scan the dashboard will never show stops being a log line nobody reads. Plugin hooks run under a "run plugin hooks" child span with the hook name and the number of plugins called. No span starts for a hook nobody implements. A hook that returns an error marks that span plugin_hook_failed and adds an exception event with the error's Go type. The scanned text, findings, tenant and app never go on a span, and error messages never do either: drivers and plugins can quote the values they failed on. The layers are still TODO, so they have no spans yet. When one lands it starts a child span from the scan's context. The Forge extension now hands the app logger to the engine, and the engine hands its logger to the plugin registry. Until now both stayed on a no-op logger, so hook errors and failed scan writes were never logged anywhere.
RetentionExecute now runs the purge under a "purge pii tokens" span: the request audit, the token delete and the completion audit. It records how many tokens the preview selected and how many the delete removed. When a step fails, the status names it (audit_failed, delete_failed or completion_audit_failed), so you can tell "nothing was deleted" apart from "tokens are gone but the audit trail is missing" without reading logs. The permission and preview checks stay outside the span. Forge's dashboard dispatcher already traces and counts every intent with its error code, so a refused purge is visible there. The preview gets no span of its own for the same reason: it is one store read, which grove traces. The subject, scope and token IDs never go on the span, and neither do audit adapter error messages.
The engine records three metrics when you give it a go-utils metrics sink
with the new engine.WithMetrics option. The Forge extension passes
app.Metrics() for you, so a Forge app gets them on /_/metrics with no
setup:
forge_shield_scans_completed_total{direction, decision}
forge_shield_scan_duration_seconds{direction, decision}
forge_shield_plugin_hooks_failed_total{hook}
direction is input or output, decision is allow, block, flag or redact,
and hook is one of the 15 plugin hooks in snake case (scan_started and so
on). There are no tenant, app, scan or plugin labels. Pass
engine.WithMetrics(nil) through WithEngineOption to turn them off.
This replaces the observability plugin's 14 unlabelled counters. Those had
dotted names (shield.scan.started and the rest). Built with
NewMetricsExtension they went to a private collector and were never
exported at all. Built from app.Metrics() they scraped as
forge_shield_scan_started and friends, typed as gauges because the names
lacked _total, and only the started and completed pair ever moved.
BREAKING CHANGE: the forge_shield_scan_started style series are gone. If
you scrape them, move to forge_shield_scans_completed_total. The
observability package still compiles, but MetricsExtension is deprecated,
implements no hooks and has lost its exported counter fields. Drop it from
your plugin list.
Adds an observability guide that lists every span with its attributes and error classes, every metric with its scraped forge_ name and label values, what never goes on either, and a table that maps the old shield.scan.* counters to the new series. The footer's Observability link now points at it, and the README gets a short Telemetry section with the same tables. The getting started, Forge extension, full example, configuration, architecture and custom plugin pages stop telling you to register observability.NewMetricsExtension(). The Forge extension wires metrics itself, and outside Forge you pass engine.WithMetrics. They also stop saying hook errors are dropped silently, because the registry now logs and counts them. AGENTS.md describes internal/telemetry and the rules for adding a span or a metric.
MetricsExtension lost its exported counter fields and its On* hook methods in this release. Code that only builds and registers the plugin still compiles, but code that read a field or called a hook method directly doesn't, so AGENTS.md and the package reference now say so.
juicycleff
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Run a scan in a Forge app and Shield now shows up in your traces and on
/_/metrics. You don't wire anything: the extension hands the engineapp.Metrics()and the app logger.curl -s localhost:8080/_/metrics | grep forge_shield_Outside Forge, pass
engine.WithMetrics(m)and install a tracer provider. The six layers are still TODO, so nothing here is per layer. When one lands, it starts a child span of the scan withshield.scan.layer.This also re-pins forge to v1.12.9 and grove (with its mongo, pg and sqlite drivers) to v1.8.0, so every store query gets a grove
db.clientspan under these.Breaking if you scrape the old counters
The
observabilityplugin's 14 dotted counters are gone. Built withNewMetricsExtension()they never left a private collector. Built fromapp.Metrics()they scraped asforge_shield_scan_startedand friends, typed as gauges. Useforge_shield_scans_completed_total. The package still compiles, butMetricsExtensionis deprecated, implements no hooks and has lost its counter fields, so take it out of your plugin list.Spans
scan contentshield.scan.id,.direction,.decision,.blocked,.findings,.persisted(with a store)run plugin hooksshield.plugin.hook,.count,.failurespurge pii tokensshield.retention.tokens_selected,.tokens_deletedA failure sets a short status class (
persist_failed,plugin_hook_failed,audit_failed,delete_failed,completion_audit_failed) and anexceptionevent carrying only the error's Go type. Scanned text, findings, tenants, apps, the subject and error messages never go on a span or a label.Metrics
forge_shield_scans_completed_totaldirection(input, output),decision(allow, block, flag, redact)forge_shield_scan_duration_secondsdirection,decisionforge_shield_plugin_hooks_failed_totalhook(the 15 hooks in snake case)Forge's dispatcher already spans and counts every dashboard intent, so we added nothing there. The retention preview is one store read that grove traces, so it gets no span either.
One fix rides along. Even inside Forge, the engine and plugin registry sat on a no-op logger, so hook errors and failed scan writes were never logged. Now they are.
Release
After that, fabriq and cortex (root and
cortex/extension) re-pin.Testing
go test -race -count=1 ./..., plus the store conformance suite against throwaway Postgres 17 and Mongo 7 containers, with nothing skipped.tracetest.SpanRecordercover names, kinds, parentage, attributes and error classes. They also pushalice@example.comthrough the scan text, tenant, subject and error messages and check it lands on no span, event or status.forge.Newapp, scraped on/_/metrics, checks names, types, labels and buckets.