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.

[wu-sheng]

Thanks for the update — this revision is a clear step forward: MockCoreModuleProvider is wired, GenAIContextResolver moved into library-util so the Zipkin/OTLP duplication is gone, and there are real unit tests now.

I went through the whole branch. Most comments below are ordinary review notes, but two need a decision before this can merge, and one is a question I need answered.

1. The module is ON by default and cannot boot. application.yml sets selector: ${SW_AI_EVALUATION:default}, so AIEvaluationProvider.prepare() always runs and throws ModuleStartException because the shipped ai-evaluation.yml has an empty endpoint/api-key. A stock OAP will not start. Note your own server-starter/pom.xml comment says "disabled by default in application.yml" and SWIP-16 step 1 says "Enable the ai-evaluation module" — so the intent was already OFF; the YAML just doesn't match it. Please flip the selector default to empty. Separately, please also make the provider degrade gracefully (log a warning and stay inactive) when the judge config is absent — an operator who enables the module but forgets the API key should lose this one optional feature, not the whole server.

Once the module is opt-in, the sampleRate default of 1000000 (100%) is fine, since enabling it is then an explicit act.

2. Question — was the GenAI provider-resolution precedence change intentional? Consolidating the two resolution paths into GenAIContextResolver silently changed behaviour for the Zipkin path, and I can't find a reason for it in the PR. Details in the inline comment; I'd like to understand the reasoning before deciding how to proceed.

The rest are individual findings inline. The ones I'd treat as must-fix are the decimal idToHexString, the JSON valueType always being dropped, the spanId filter mapped to span_index, and the asymmetric MeterEntity id change.

[wu-sheng]

Blocking: a stock OAP will not boot.

${SW_AI_EVALUATION:default} selects the default provider unconditionally, so prepare() runs -> createJudgeProvider() -> new OpenAICompatibleProvider(judge) -> validate(), which throws ModuleStartException("AI evaluation judge config [endpoint] is required.") because ai-evaluation.yml ships endpoint/api-key as empty.

This also contradicts server-starter/pom.xml:62 ("disabled by default in application.yml") and SWIP-16's operator workflow ("1. Enable the ai-evaluation module in OAP").

Please default the selector to empty, and additionally have the provider log-and-stay-inactive rather than abort startup when the judge config is missing.

[wu-sheng]

Once the module is opt-in (see above) this default is acceptable.

Leaving it at 1000000 while the module is on by default would mean the first operator who sets an endpoint immediately sends every GenAI span in the fleet to a paid judge model, with up to 16 KB input + 16 KB output each. Please make sure these two land together.

[wu-sheng]

Question: was this reordering intentional?

The two paths this replaces had different precedence, and the merged order matches neither:

	old OTLP	old Zipkin	new resolve()
1	gen_ai.provider.name	gen_ai.provider.name	gen_ai.provider.name
2	prefix trie	gen_ai.system	prefix trie
3	—	prefix trie	gen_ai.system

Net effect: the OTLP path gains a gen_ai.system fallback it never had (fine), but the Zipkin path loses the priority it had. A span with gen_ai.system=azure_openai and gen_ai.response.model=gpt-4o used to report provider azure_openai; it now reports openai, because the trie matches the gpt- prefix first. On upgrade, existing GenAI provider service entities and their metrics silently re-key.

On the merits I think the old Zipkin order was right: gen_ai.system is an explicit declaration by the instrumentation, whereas the trie is a guess inferred from the model-name prefix — the declaration should win.

GenAIContextResolverTest.shouldPreferModelMatchOverLegacySystem now asserts the new order, so CI will never flag this. If the change was deliberate, please explain the reasoning and add a changes.md note, since it is a user-visible behaviour change. If it was an accident of the refactor, please restore declared -> gen_ai.system -> trie and update that test.

[wu-sheng]

new BigInteger(1, id.toByteArray()).toString() is radix 10, so this returns a decimal string despite the method name and the new OTLPSpanReader javadoc ("encoded as lowercase hex").

A trace id of 0af7651916cd43dd... comes back as 14556450186841972..., which will never join against the hex trace ids stored everywhere else (OTLP/Zipkin traces, GenAIEvaluationRecord.traceId).

.toString(16) alone isn't enough either — BigInteger drops leading zero bytes, so the result needs zero-padding to 32/16 chars. Suggest formatting the bytes directly.

Both traceId() and spanId() are currently unused, which is why nothing caught this. Adding abstract methods to OTLPSpanReader is also a breaking change for any out-of-tree implementer — a default method would avoid that.

[wu-sheng]

Tasks declared valueType: JSON are always silently dropped.

The shipped system prompt tells the judge "value must be a valid JSON object". When the model complies and returns {"MyTask":{"value":{"a":1},"reason":"..."}}, element.getAsString() on a JsonObject throws UnsupportedOperationException. That is swallowed by the catch (RuntimeException ignored) above and the task is skipped with only a warn.

If every configured task is JSON, results is empty and SpanAIEvaluationStrategy then raises JudgeModelException(INVALID_RESPONSE) for a perfectly valid response.

Suggest returning element.toString() for non-primitive elements. Note also validateJson is effectively a no-op — JsonParser.parseString is lenient and accepts any bare string, so it won't reject a non-object.

[wu-sheng]

Making this public adds an API that is order-dependent and not safe after boot:

• AnalyzerModuleProvider.start() does segmentParserService.setListenerManager(listenerManager()), constructing a new SegmentParserListenerManager. A caller invoking this from prepare() gets an NPE, and one invoking it before that line has its factory silently discarded. It works today only because BootstrapFlow happens to order analyzer before ai-evaluation.
• SegmentParserListenerManager backs the list with a plain LinkedList that getSpanListenerFactories() iterates for every segment, so any post-boot call risks ConcurrentModificationException.

The existing SpanListener ServiceLoader SPI — which this same PR already uses for the Zipkin path — is the intended mechanism. Could the native-trace path use it too, rather than adding this?

[wu-sheng]

setModelName computes modelId from the mutable providerId field, so it silently requires setProviderName to have been called first.

SpanAIEvaluationStrategy.persistResults happens to call them in that order. Swap those two lines, or add any other construction site that sets the model first, and every record is written with modelId built from a null provider — no error anywhere, the query key just silently diverges.

Nothing in the class signals this. Suggest computing modelId in a builder/factory that takes both values, so the ordering can't be got wrong.

[wu-sheng]

This Reader is never closed — no close(), no try-with-resources, and the catch (FileNotFoundException) path can't close it either.

GenAIPricingConfigLoader in the same feature area already does try (Reader reader = ResourceUtils.read(CONFIG_FILE)); please match it.

[wu-sheng]

This PR ships a user-facing config/ai-evaluation.yml (judge endpoint / model / api-key / timeout / retries / temperature / max_tokens, system prompt, level rules, tasks), five SW_AI_EVALUATION_* environment variables, a new GraphQL query and the gen_ai_model_evaluation_score_ppm metric — but the only documentation is this SWIP.

Per the project convention, a SWIP is a record of what was proposed and agreed, not a description of current behaviour; operator docs are the only place a reader should be sent for current truth. Please add a page under docs/en/setup/backend/ covering how to enable and tune the feature, and register it in docs/menu.yml.

There is also no e2e coverage under test/ for the new pipeline.

[wu-sheng]

The operator page and menu entry are now present, thanks. One required prerequisite is still missing from the page: the Java agent defaults Spring AI input/output message collection to false, and the evaluator requires both attributes. Please document SW_PLUGIN_SPRINGAI_COLLECT_INPUT_MESSAGES=true and SW_PLUGIN_SPRINGAI_COLLECT_OUTPUT_MESSAGES=true, including that enabling them sends captured content to the configured judge. The Virtual GenAI E2E needs the same settings.

[wu-sheng]

Nit: new changelog entries are appended at the end of their section, so the section reads in the order changes landed. This one is inserted at the top of #### UI (and the OAP Server entry lands mid-section rather than at the end). Please move both to the end of their respective sections.

[wu-sheng]

Please fix the license header.

[wu-sheng]

The optional-module integration and default BanyanDB write path are blocking. The remaining comments cover query invariants and storage schema correctness.

[wu-sheng]

[P1] Gate this listener on the optional AI-evaluation module

ai-evaluation is disabled by default, but its JAR and SPI factory remain on the classpath. TraceAnalyzer.createSpanListeners() invokes this factory for every native segment, and moduleManager.find(AIEvaluationModule.NAME) throws ModuleNotFoundRuntimeException before any span filtering occurs. Consequently, stock configuration rejects all native-agent trace segments, not only GenAI spans. Please gate SPI factories by required modules, analogous to SpanListenerManager, or avoid registering/creating this listener when ai-evaluation is unavailable.

[wu-sheng]

[P1] Preserve nullable native spanIndex in BanyanDB

spanIndex is the raw native SkyWalking SpanObject.spanId: an int32 starting at zero and unique only within its segment. OTLP/Zipkin instead use a hexadecimal string spanId and intentionally leave spanIndex null. This nullable Integer reaches BanyanDBConverter.buildTag, which dereferences it through ((Number) value).longValue(), so every OTLP/Zipkin evaluation fails before the BanyanDB write. Please make the Integer conversion null-safe and pass null through to longTagValue; do not introduce a sentinel.

[wu-sheng]

[P2] Enforce the evaluation type/value pair for value-based queries

Please enforce that an evaluation value and its valueType travel together. In particular, sortBy=SCORE_VALUE should only be accepted with valueType=SCORE; incompatible or missing type/value combinations should be rejected at the query boundary and reflected in the GraphQL contract. Currently ES/BanyanDB sort the shared raw numeric column, which also contains BOOLEAN values, while JDBC's final comparator treats BOOLEAN records as having a null score. This makes identical queries return backend-dependent ordering and page contents.

[wu-sheng]

[P2] Size model_id for a complete ServiceInstanceID

modelId is produced with ServiceInstanceID.buildId(providerId, modelName), but this column is limited to 150 characters. With the permitted 70-character provider and model names, the encoded ID can reach 195 characters. JDBC may reject or truncate a valid ID that ES/BanyanDB store successfully, and truncation also prevents correct model-name decoding. Please use the established service-instance ID capacity, such as 250 or 512 characters.

[wu-sheng]

The increase to 250 fixes the ASCII example, but the encoded-ID capacity is still incomplete. NamingControl limits Java characters, while IDManager Base64-encodes UTF-8. A valid 70-character CJK name becomes 280 encoded characters, so a service/provider ID is about 282 characters and the corresponding model ServiceInstanceID is about 563. SERVICE_ID and PROVIDER_ID also remain at 150. Please size all three columns for the encoded representation, or enforce an equivalent encoded-byte limit before building the IDs.

[wu-sheng]

[P2] Store evaluation results as long storage-only text

The 4096-character limit has no corresponding parser, provider, or protocol constraint, and this value is not used as a query condition. Blind substring truncation can turn a previously validated JSON object into invalid JSON. Please give this storage-only field a documented long-text capacity and preserve the complete value. The existing 50,000-character AlarmRecord.snapshot is a same-stream precedent across JDBC, Elasticsearch, and BanyanDB. If a hard maximum is required, reject an over-limit JSON result rather than truncating it.

[wu-sheng]

[CI] Add the ASF header to this SPI descriptor

This new service descriptor lacks the commented ASF license header used by the other META-INF/services files. It is the sole invalid file reported by license-eye and currently fails the required License Header check.

[wu-sheng]

[P1] Enable message capture so this E2E actually reaches the judge

The pinned Java agent defaults SW_PLUGIN_SPRINGAI_COLLECT_INPUT_MESSAGES and SW_PLUGIN_SPRINGAI_COLLECT_OUTPUT_MESSAGES to false, while SpanAIEvaluationStrategy accepts only spans containing both message tags. The spring-ai-examples environment does not set them, so evaluation exits before judge(), and both Virtual GenAI attempts ended with an empty evaluation metric.

Please set both variables to "true" on spring-ai-examples. Once enabled, the existing 800000-PPM metric and SCORE/BOOLEAN record assertions exercise judge invocation through response parsing, level resolution, persistence, and GraphQL. One remaining coverage limitation is that the mock endpoint ignores the request body and headers, so it does not verify the configured model, authorization, system prompt, captured messages, or task instructions; please capture and assert the minimum request fields if request construction is intended to be covered end to end.

[wu-sheng]

[CI] Update the storage config-dump fixture with this default

This appends gen-ai-model to meterAnalyzerActiveFiles, but test/e2e-v2/cases/storage/expected/config-dump.yml still expects the previous list. Storage MySQL passed its other 74 checks and failed on this exact mismatch in both attempts. Please update the expected value together with this change.

[wu-sheng]

[P2] Validate the discriminator-specific address fields before querying

All three DAOs silently branch on type and ignore or misapply incompatible fields. For example, SKYWALKING_NATIVE plus spanId and no spanIndex ignores spanId and returns the broader trace or segment result; OTLP plus spanIndex applies the native span_index predicate and returns nothing. Please reject fields belonging to the other addressing scheme at the query-service boundary. When a native spanIndex is supplied, also require segmentId because the index is only segment-local.

[wu-sheng]

About #13943 (comment), I have another doubt from user experience, users are nearly impossible to get all trace segment/span IDs, only trace ID is common.

What is your UX workflow?