SWIP-16 Support LLM-as-Judge on Top of GenAI Observability

[peachisai]

• If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #.
• Update the CHANGES log.

[wu-sheng]

This looks dangerous, could you ref to existing app log?
I feel we have a uuid kind of thing?

[peachisai]

This looks dangerous, could you ref to existing app log? I feel we have a uuid kind of thing?

Fixed

[wu-sheng]

Thanks for the work here — the module structure, the PPM sampling and pairing the records with a MAL metric are all the right shape.

I want to focus this round entirely on the query API, because query-protocol is a published contract for the UI and it is much cheaper to change now than after release. Five things, roughly in priority order. Nothing below is about code style.

1. The UI cannot render the page SWIP-16 specifies

GenAIEvaluationRecord returns only opaque ids — no serviceName, no providerName, no modelName, no operationName. To display "openai / gpt-4o" the UI would have to base64-decode SkyWalking's internal ID encoding client-side, which no other surface asks of it.

The house convention is the opposite — type Log returns both, and LogQueryService enriches the names server-side via IDManager.ServiceID.analysisId(...):

type Log {
    serviceName: String
    serviceId: ID
    serviceInstanceName: String
    serviceInstanceId: ID
    endpointName: String
    endpointId: ID
    ...
}

Worse, this is not fixable later without a data migration: AIEvaluationContext already carries serviceName, serviceInstanceName and operationName, and persistResults writes none of them. Note also that serviceId here is the GenAI provider and serviceInstanceId is the model — the calling application is not stored at all, so "show me judge scores for my chatbot service" cannot be asked now or later.

Please add serviceName/serviceInstanceName to the type (enriched in the query service, zero storage cost), and persist providerName, modelName, operationName and the caller's service name as real columns.

2. The chart → records drill-down is not expressible

The whole reason to pair a record list with a metric is that a user clicks a dip in the score chart and gets the responses behind it. gen_ai_model_evaluation_score_ppm aggregates by provider_name / model_name / task_name. The record query can filter by none of the three. There is no query that answers "show me the responses behind this point".

(Separately, SWIP-16 names the metric gen_ai_evaluation_score_ppm, but the rule emits gen_ai_model_evaluation_score_ppm — metricPrefix: gen_ai_model_evaluation + name: score_ppm. Worth fixing in the doc.)

3. The primary filters do not work on Elasticsearch, and the trace drill-down silently returns nothing on BanyanDB

Column.storageOnly() is documented as "The column is just saved, never used as a query condition", and StorageEsInstaller.createMapping stamps "index": false for it, plus "doc_values": false unless @ElasticSearch.EnableDocValues is present.

In GenAIEvaluationRecord these are all storageOnly = true: service_id (L71), service_instance_id (L76), segment_id (L80), span_id (L83), value_type (L93), value (L96), reason (L104), judge_model (L107). Yet GenAIEvaluationRecordQueryEsDAO term-queries service_id (L95), service_instance_id (L98), segment_id (L105), span_id (L108), and any whitelisted tag key (L117).

segment_id, span_id, value_type, value, reason and judge_model have no EnableDocValues, so they get index:false, doc_values:false and a term query on them cannot work on any ES version. service_id/service_instance_id keep doc values, so behaviour there depends on the ES version — which is its own problem, since we support ES 7/8/9 and OpenSearch and the same GraphQL document would behave differently across them.

On BanyanDB the drill-down fails a different way:

query.and(eq(GenAIEvaluationRecord.SPAN_ID, (long) relatedTrace.getSpanId()));  // L89

That binds an int64 against a tag registered from a String column (ES and JDBC both use String.valueOf(...)). It returns an empty list, not an error — indistinguishable to the user from "the judge said nothing about this span".

Suggested fix: drop storageOnly from every column a DAO filters on (service_id, service_instance_id, segment_id, span_id, value_type, judge_model — the same set AbstractLogRecord keeps indexed), keep it only on the 4096-char value and reason and remove those two from QUERYABLE_TAG_KEYS, and derive the whitelist from one shared constant so the annotation and the whitelist cannot drift apart again.

4. "The 20 worst-scoring responses" cannot be asked

value is a String for all four value types, and queryOrder: Order is only a direction — all three DAOs hard-wire the sort column to evaluation_time. So neither "rank the worst outputs" nor "scores below 0.5" is expressible, and that is the primary triage entry point for judge data.

We already have precedent for both halves — enum QueryOrder { BY_START_TIME, BY_DURATION } in trace.graphqls, and RecordCondition{topN, order} in record.graphqls. Suggest a real numeric score column (indexed, @BanyanDB.EnableSort), scoreValue: Float on the type, minScore/maxScore on the condition, and an explicit sort field. Please also give BanyanDB's OrderBy an explicit column — today it passes a bare new AbstractQuery.OrderBy(Sort.DESC) and only matches evaluation_time by virtue of @BanyanDB.TimestampColumn, which is an accidental coupling.

5. The filter dropdowns cannot be populated

Task names and level bands are operator config in ai-evaluation.yml, and nothing in the API exposes them — so the "filter by task / filter by level" controls SWIP-16 promises can only be built by hardcoding today's defaults, which breaks the moment an operator renames a band or adds a task.

They are also reachable only through the generic tags list, keyed by snake_case storage column names (evaluation_level) that do not match the camelCase response fields (evaluationLevel) — and a wrong key behaves three different ways: empty page on ES and JDBC, ErrTagNotDefined on BanyanDB. A UI cannot write one error path for that, and an empty page for a typo is a silent wrong answer.

Every other family that offers tags ships the autocomplete companions for exactly this reason — queryLogTagAutocompleteKeys/Values, queryTraceTagAutocompleteKeys/Values, queryAlarmTagAutocompleteKeys/Values. Please promote taskName, evaluationLevel and judgeModel to typed condition fields (the way alarm.graphqls promoted ruleNames), and either add the autocomplete queries or expose the configured tasks/bands directly.

---

Two smaller notes while the API is open: supportGenAIEvaluationRecordQueryByKeywords: Boolean! returns the DAO interface default false, is overridden by no storage, and the condition has no keyword field to pair with it — please either implement keyword search over the judge's explanation (mirroring LogQueryEsDAO) or drop the flag rather than ship a public non-null field that can never become meaningful without a breaking rename. And errorReason on the result wrapper is never set by anything, while the one real error path throws.

Lastly — nothing in the tree executes queryGenAIEvaluationRecord: no UT, no storage IT, no e2e. That is almost certainly why the three backends disagree. One e2e case per storage that writes an evaluation and reads it back with a relatedTrace drill-down and a task/level filter would have caught 3 and most of 5.

[wu-sheng]

Thanks for taking SWIP-16 this far — the pipeline shape is right, and keeping the judge call off the trace-analysis critical path is the correct core decision.

I've done a full pass over the storage structure, query logic, and the ingestion-to-judge pipeline. A lot below, so I've ordered it by what unblocks what. Everything is from reading the source, because the branch doesn't compile, so none of it has run on CI.

Heads-up: the query protocol changed

Reviewing this surfaced problems in the merged schema, so I raised and merged skywalking-query-protocol#164. The contract you're implementing against has moved — please bump the submodule pointer early, since several mismatches below only become visible once you do.

1. Blockers

Build is red.

AIEvaluationProvider.java:[27,60] package org.apache.skywalking.oap.meter.analyzer.v2.dsldebug does not exist

The package is …meter.analyzer.v2.dsl.debug. 225 of 230 checks fail behind this. See §2 — the cleanest fix removes the import entirely.

OAP doesn't boot out of the box. application.yml enables ai-evaluation by default, and prepare() → OpenAICompatibleProvider.validate() throws ModuleStartException when endpoint/api-key are blank — which is what ai-evaluation.yml ships. Every stock install and every existing e2e fails at startup. A feature that spends money per span should be opt-in: default the selector off, and construct no judge when unconfigured.

MAL rules and config never reach the distribution. ai-evaluation.yml and gen-ai-evaluation-rules/ aren't in apm-dist/src/main/assembly/binary.xml, so Rules.loadRules resolves inside the starter jar and Files.walk throws. The file whose header says "judge model and tasks are maintained here" also isn't in the tarball for an operator to edit.

MockCoreModuleProvider doesn't register GenAIEvaluationRecordQueryService. CoreModule.services() gained it, so profile-exporter throws ServiceNotProvidedException at boot. This is the case in CLAUDE.md tip #11 — check every extends CoreModuleProvider.

2. Module placement — please reconsider

ai-evaluation is a new top-level Maven module, sibling to ai-pipeline, while analyzer/gen-ai-analyzer already recognizes GenAI spans and resolves provider/model. The separate ModuleDefine is justified — evaluation should be disableable independently. The Maven placement isn't, and it's what produced the duplication:

• GenAISemanticAttributes re-transcribes GenAITagKeys, with inconsistent constant names for identical values.
• GenAIContextResolver re-implements GenAIMeterAnalyzer's provider resolution — and diverges from it. On the native path the existing analyzer does provider.name → matcher, while the new resolver inserts a gen_ai.system fallback. For Azure OpenAI (gen_ai.system=az.ai.openai, model gpt-4o) the analyzer yields openai and the evaluator yields az.ai.openai — traffic and cost land on one virtual GenAI service, the quality score on another.
• GenAIEvaluationAnalysisListener sits in agent-analyzer, not here, so the two halves of the pipeline are already split.

Suggested: keep the ModuleDefine, move the Maven module under analyzer/, move the native listener into it so the dependency points ai-evaluation → agent-analyzer (not the reverse), and lift the tag constants plus one resolveProvider(tags) into library-util/…/genai/ where GenAIModelMatcher already lives.

Related: the MAL rule shouldn't have its own catalog. gen-ai-evaluation-rules is registered in neither Catalog nor MalRuleEngine.CATALOGS, so it's the only MAL rule in the product that can't be hot-updated or debugged. It's one labeled metric from an in-process producer — exactly what meter-analyzer-config already holds (continuous-profiling.yaml, network-profiling.yaml). Move it there and dispatch through IMeterProcessService.converts() instead of loading rules and holding a private List<MetricConvert>. That also deletes the dsl.debug import causing the build failure, and the dist-packaging problem for the rules directory.

3. Data model

Worth settling before the rest, since query and dashboard sit on top.

Column naming. EVAL_NUMBER_VALUE = "evaNumberValue" and EVAL_STRING_VALUE = "evalStringValue" are the only camelCase column names in core/analysis/ — and one has a typo (eva vs eval; the constant is spelled correctly). Please use eval_number_value / eval_string_value, field evalNumberValue.

model_id doesn't identify the model. The GenAI entity model is already defined: provider is a service in Layer.VIRTUAL_GENAI, model is a service instance of it (GenAIProviderAccess / GenAIModelAccess). toEntityId(modelName) builds a ServiceID from a model name — matching nothing. Same model name under two providers collides, and toStoredModelId() exists only to translate the UI's real ServiceInstanceID into the synthetic form. Store the canonical ServiceInstanceID.

Span id. Integer.parseInt(context.getSpanId()) runs before the record is written. AIEvaluationSpanListener supplies a 16-char hex Zipkin id, so every OTLP/Zipkin evaluation is discarded after the judge call is billed — the exception is swallowed by AIEvaluationService.evaluate(). #164 introduces GenAITraceRef for this: a SKYWALKING_NATIVE | OTLP discriminator, with spanIndex: Int for native (it's a segment-local index, not an id) and spanId: String for OTLP/Zipkin. Storage becomes a ref_type column plus a String span-identifier column, and the parse disappears.

Score scale. getScoreValue() returns evaNumberValue / (double) SCORE_SCALE, but the schema declares scoreValue: Long at ppm — so every non-integral score fails scalar serialization and only 0.0 / 1.0 survive. Separately minScore/maxScore are Long ppm in the schema but Double in Java, and the DAOs then multiply by 1e6 again, so a UI sending 860000 produces a bound of 860000000000 and matches nothing. The write path is already right (toScoreValuePpm uses BigDecimal); keep Long end to end and stop converting on the read path.

storageOnly columns used as filters. value_type and segment_id are declared storageOnly = true — which Column's javadoc defines as "never used as a query condition" — yet both are filtered. On Elasticsearch that's query_shard_exception; on JDBC it silently works. value_type is the filter the schema documents as primary.

Small ones: task_name at length = 512 is longer than any shipped task name and pushes the column past the string-index threshold — 64 is plenty. reason and evalStringValue are 4096 but hold unbounded LLM text with no truncation on the write path. setServiceName skips NamingControl while the two adjacent lines apply it, so a service name over 70 chars gets an id the UI can never match.

4. Pipeline

Buffering and backpressure. new ThreadPoolExecutor(4, 4, 0, SECONDS, new ArrayBlockingQueue<>(100)) is hardcoded — no configuration, unnamed non-daemon threads, never shut down. Total admission is 104, and since each task makes a blocking HTTP call the sustained rate is 4 / judge_latency, well under 1 span/sec. Please use library-batch-queue with BufferStrategy.IF_POSSIBLE (returns false rather than constructing an exception), with bufferSize and consumer threads configurable. Note the default PartitionSelector.typeHash() collapses a single-element-type queue onto one partition — a custom selector is needed.

Drops must be counted, not logged. The rejection path logs warn with the exception, so overflow prints a stack trace per dropped span — the trace is identical every time and carries nothing. There's no telemetry in the module at all, so coverage can fall from 100% to 1% with no signal but log volume. That silently invalidates the score metric, because what survives is no longer the uniform sample the ppm policy computed. Please add MetricsCreator counters (adding TelemetryModule.NAME to requiredModules()), with reasons naming the operator-actionable condition — sampling / pipeline capacity / incomplete span — and adopt them in otel-rules/oap.yaml. Judge errors belong there too, split rejected / timeout / invalid_response: the first says fix credentials or quota, the last says the model is ignoring the output contract. Today both are the same IOException.

Validate before buffering. support() only checks that traceId and spanId are non-empty; the real gate, validLLMCallSpan(), runs on the worker thread — so non-judgeable spans consume a queue slot, a thread and a dedup key before being discarded. findStrategy() returning null already gives a free rejection path. And the gate itself is too narrow: gen_ai.operation.name == "chat" excludes text_completion and generate_content, which are inference calls carrying both message fields. The sufficient predicate is the next line — input and output messages present.

Dedup key. traceId + "-" + spanId omits segmentId. spanId is unique only within a segment, so two segments of one trace both start at 0 and the second evaluation is silently dropped. segmentId is already in the context.

Timestamps. evaluationTime = System.currentTimeMillis() at judge completion becomes both the record's time_bucket and the MAL sample timestamp, so queue depth directly displaces the score series from the traffic it describes. AIEvaluationContext.endTimeMillis is populated by both listeners and never read (as are startTimeMillis, serviceInstanceName and operationName). Stamp from the span.

Default sample rate is 100% (1000000), so every GenAI span triggers a paid judge call out of the box. The env var is also misspelled — SW_AI_EVALUTION_SAMPLE_RATE.

Parser is all-or-nothing. JsonParser.parseString(content).getAsJsonObject() is unguarded — a judge returning markdown-fenced JSON (common) throws — and one missing task key discards all four results after the call is billed. Please strip fences and validate per task, persisting what parsed.

Prompt construction. Input/output messages are concatenated with no truncation or size ceiling; max_tokens: 100000 caps only the response. And the judged model's own output is injected unescaped ahead of the task list, so text in a monitored application's response can restate the tasks or dictate the verdict — which is then stored as ground truth and aggregated into a dashboard metric. Please delimit and escape the untrusted region and put instructions after it.

No 429 / Retry-After handling. Any non-2xx becomes IOException, indistinguishable from a 400, and there's no retry — so a rate-limit burst discards every evaluation in flight after each was billed.

5. Configuration

Please align the judge config with Horizon's shipped schema (apps/bff/src/config/schema.ts), which already solved this vendor-neutrally: provider: openai-compatible | bedrock naming a transport rather than a vendor, base-url for OpenAI-shaped endpoints, optional region for Bedrock falling back to AWS_REGION, and model carrying the exact Bedrock/inference-profile id. Horizon deliberately omits temperature and token caps — "the gateway / provider / model owns them" — worth matching. Env prefix should be SW_* per project convention, and the api-key needs a redaction story.

On the two config files: the judge block is wiring, not content, so the split as drawn isn't justified — and prepare() discarding the module-bound config to hand-carry sampleRate shows the seam. Suggest application.yml takes selector, sampleRate, buffer size, consumer threads and the judge block, while ai-evaluation.yml keeps system-prompt, level and tasks — the content an operator authors, matching the alarm-settings.yml precedent.

One bug there: level.boolean uses unquoted true: / false: keys, which SnakeYAML parses as Boolean, while the loader looks them up as String. BOOLEAN tasks always fall back to undefined.

6. Tests

test/e2e-v2/cases/otlp-virtual-genai/ already emits GenAI spans over OTLP against BanyanDB — the natural host, and exactly the path the span-id bug breaks. Run it at 100% sample rate against a mock judge returning a fixed OpenAI-shaped choices[0].message.content. Two properties worth designing in: the stub must return exactly the configured task keys, which makes it a contract test for the prompt/parser pair; and a deterministic score so the assertion pins 0.86 → 860000 in both the record and the metric. With a native-path case too, e2e would cover five of the eight blockers above.

---

Happy to go through any of these in more detail. I'd suggest sequencing as: get the build green, then the data model (naming, trace ref, score scale, model_id), then module placement and the MAL rule move — the pipeline and config items are largely independent of each other once those land.

[wu-sheng]

Follow-up on the buffering point in my earlier review — library-batch-queue now has what this pipeline needs, so the hand-rolled executor can go away rather than being tuned.

#13979 added ThreadPolicy.ioBound(N) for exactly this shape: a queue whose consumers spend most of their time blocked on a remote call. It runs the drain loops on virtual threads where the runtime provides them (JDK 25+, which the shipped OAP image is) and on N platform threads otherwise, with identical semantics on both paths.

Suggested replacement for AIEvaluationService's executor

BatchQueueManager.create(
    "AI_EVALUATION",
    BatchQueueConfig.<AdmittedEvaluation>builder()
        .threads(ThreadPolicy.ioBound(concurrency))
        .partitions(PartitionPolicy.fixed(concurrency))     // 1:1 — see below
        .partitionSelector((d, n) -> Math.floorMod(d.dedupKey().hashCode(), n))
        .bufferSize(smallPerPartition)
        .strategy(BufferStrategy.IF_POSSIBLE)
        .minIdleMs(50).maxIdleMs(500)
        .shutdownTimeoutMs(judgeTimeoutMs)
        .consumer(this::judgeBatch)
        .errorHandler((data, t) -> judgeErrorCounter.inc(data.size()))
        .build());

Four things worth knowing before wiring it up, all of them easy to get wrong:

Partitions must match threads 1:1. threadCount is clamped to partitionCount, so ioBound(50) with the default PartitionPolicy.fixed(1) silently becomes one drain loop. The drain loop is the task processor — nothing is handed off — so consumer concurrency is min(threads, partitions). Since bufferSize is per partition, keep it small: 50 partitions x 4 is 200 queued, not 50 x 50.

Don't take the default PartitionSelector. typeHash() routes by data.getClass().hashCode(), and this queue has one element type, so every item lands on partition 0 — concurrency 1 regardless of configuration. And don't key the selector on the sampling hash: sampling admits on floorMod(murmur3(traceId), 1_000_000) < sampleRate, and since any sane partition count divides 1,000,000, at low sample rates every admitted item would land on partition 0. Key it on something else — the dedup key (traceId + segmentId + spanId) works.

IF_POSSIBLE is what removes the log flood. produce() returns false when the buffer is full rather than throwing, so there is no RejectedExecutionException and no stack trace per dropped span. Count the false return with a MetricsCreator counter — that is the coverage signal the module currently has no way to report.

Keep maxIdleMs <= shutdownTimeoutMs. On the virtual path a drain task parked on its idle backoff sleeps inside the submitted task and cannot be dropped at shutdown, so a larger maxIdleMs makes every teardown log a spurious "drain loops did not finish" warning. Also raise minIdleMs well above the 1ms default — a millisecond poll interval buys nothing when the work takes seconds, and on the platform fallback each idle wake is a real context switch.

This addresses the executor half of the earlier review (hardcoded 4/4/100, unnamed non-daemon threads, never shut down, AbortPolicy with a stack trace per drop). The other half still stands independently: validate before admitting, so a span that cannot be judged never takes a slot.

One sequencing note — raising concurrency is only safe once OpenAICompatibleProvider handles 429 and Retry-After. Today any non-2xx becomes an IOException with no retry, so a burst against a rate limit discards every evaluation in flight after each was billed.