Copilot commented on code in PR #13884:
URL: https://github.com/apache/skywalking/pull/13884#discussion_r3300632232
##########
docs/en/setup/backend/configuration-vocabulary.md:
##########
@@ -333,7 +333,7 @@ It divided into several modules, each of which has its own
settings. The followi
| - | - | exportErrorStatusTraceOnly
| Export error status
trace segments through the Kafka channel.
| SW_EXPORTER_KAFKA_TRACE_FILTER_ERROR
| false
|
| health-checker | default | checkIntervalSeconds
| The period of checking
OAP internal health status (in seconds).
| SW_HEALTH_CHECKER_INTERVAL_SECONDS
| 5
|
| status | default | (selector)
| Status feature module —
hosts /status/* and /debugging/* routes on the admin-server REST host only.
Both `status` and `admin-server` are enabled by default; set SW_STATUS empty to
disable.
| SW_STATUS
| default
|
-| - | - | keywords4MaskingSecretsOfConfig
| Include the list of
keywords to filter configurations including secrets. Separate keywords by a
comma.
|
SW_DEBUGGING_QUERY_KEYWORDS_FOR_MASKING_SECRETS |
user,password,token,accessKey,secretKey,authentication
|
+| - | - | keywords4MaskingSecretsOfConfig
| Include the list of
keywords to filter configurations including secrets. Separate keywords by a
comma.
|
SW_DEBUGGING_QUERY_KEYWORDS_FOR_MASKING_SECRETS |
user,password,trustStorePass,keyStorePass,token,accessKey,secretKey,authentication
|
Review Comment:
The default value documented here was updated, but the same default is also
documented in `docs/en/debugging/config_dump.md` and it still lists the old
keyword set. Please update that page too to avoid conflicting documentation
about what is masked by default.
##########
oap-server/server-starter/src/main/resources/application.yml:
##########
@@ -746,7 +746,7 @@ status:
selector: ${SW_STATUS:default}
default:
# Include the list of keywords to filter configurations including secrets.
Separate keywords by a comma.
- keywords4MaskingSecretsOfConfig:
${SW_DEBUGGING_QUERY_KEYWORDS_FOR_MASKING_SECRETS:user,password,token,accessKey,secretKey,authentication}
+ keywords4MaskingSecretsOfConfig:
${SW_DEBUGGING_QUERY_KEYWORDS_FOR_MASKING_SECRETS:user,password,trustStorePass,keyStorePass,token,accessKey,secretKey,authentication}
Review Comment:
The default masking keyword list is duplicated elsewhere in the codebase;
updating it only in `application.yml` will leave other default paths
inconsistent. `oap-server/server-admin/status/.../StatusModuleConfig` still
defaults to `user,password,token,accessKey,secretKey,authentication`, so
deployments that rely on the module's code default (or other config templates)
won't mask `trustStorePass`/`keyStorePass` by default. Consider updating the
module config default (and any other shipped templates) to keep the effective
defaults aligned.
##########
docs/en/setup/backend/admin-api/status.md:
##########
@@ -28,7 +28,7 @@ export SW_STATUS=default # default (enabled)
status:
selector: ${SW_STATUS:default}
default:
- keywords4MaskingSecretsOfConfig:
${SW_DEBUGGING_QUERY_KEYWORDS_FOR_MASKING_SECRETS:user,password,token,accessKey,secretKey,authentication}
+ keywords4MaskingSecretsOfConfig:
${SW_DEBUGGING_QUERY_KEYWORDS_FOR_MASKING_SECRETS:user,password,trustStorePass,keyStorePass,token,accessKey,secretKey,authentication}
```
Review Comment:
This doc now shows the updated default masking keywords, but
`docs/en/debugging/config_dump.md` still documents the old default list. Please
update the config-dump doc as well so readers get a consistent default value
across docs.
##########
oap-server/server-starter/src/main/resources/application.yml:
##########
@@ -746,7 +746,7 @@ status:
selector: ${SW_STATUS:default}
default:
# Include the list of keywords to filter configurations including secrets.
Separate keywords by a comma.
- keywords4MaskingSecretsOfConfig:
${SW_DEBUGGING_QUERY_KEYWORDS_FOR_MASKING_SECRETS:user,password,token,accessKey,secretKey,authentication}
+ keywords4MaskingSecretsOfConfig:
${SW_DEBUGGING_QUERY_KEYWORDS_FOR_MASKING_SECRETS:user,password,trustStorePass,keyStorePass,token,accessKey,secretKey,authentication}
Review Comment:
This changes the boot config dump value for
`status.default.keywords4MaskingSecretsOfConfig`. The e2e expected file
`test/e2e-v2/cases/storage/expected/config-dump.yml` still asserts the old
value (`user,password,token,accessKey,secretKey,authentication`), so e2e will
fail unless that expectation is updated too.
##########
docs/en/changes/changes.md:
##########
@@ -285,6 +285,7 @@
* Fix: `avgHistogramPercentile` / `sumHistogramPercentile` meter functions
reported the smallest finite bucket boundary (e.g. `10` for OTel
`gen_ai_server_request_duration` whose `le` is rewritten from `0.01s` → `10ms`)
for every rank when no samples were observed in any bucket. The percentile
loop's `count >= roof` check matched on the first sorted bucket because both
sides were `0`. `calculate()` now short-circuits to `0` for every rank when the
windowed total is `0`.
* Fix: MAL `expPrefix` now applies to every metric source in `exp`, not just
the leading one. Previously the prefix was spliced after the first `.`, so
secondary metrics inside arguments (e.g. the divisor in
`a.sum(['s']).safeDiv(b.sum(['s']))`) silently skipped the prefix — a rule like
envoy-ai-gateway's `request_latency_avg` (`sum / count`) would tag-rewrite only
the dividend. The injection is now AST-aware: every bare-IDENTIFIER metric
source is wrapped, while downsampling-type constants (`SUM`, `AVG`, `LATEST`,
`SUM_PER_MIN`, `MAX`, `MIN`) are skipped.
* Add `@Stream(allowBootReshape = true)` opt-in for additive boot-time reshape
of BanyanDB streams / measures. Code-defined stream classes (e.g.
`AlarmRecord`) can now annotate their schema as eligible for in-place additive
update at OAP boot — a new `@Column` is appended to the live tag-family /
fields via `client.update` instead of being silently rejected with
`SKIPPED_SHAPE_MISMATCH` (which previously forced operators to drop the measure
/ stream and lose historical rows). Additive includes both new tags / fields
**and** relocating an existing tag between families when a `@Column`'s
`storageOnly` flag flips (e.g. `id1` moving from `storage-only` → `searchable`
when it becomes indexed). The opt-in is per-stream and gated by an
`isPurelyAdditive` shape diff: tag type changes, tag drops, kind flips
(tag↔field), entity / interval / sharding-key changes, and field re-typing
still skip with `SKIPPED_SHAPE_MISMATCH`, so identity-breaking edits remain
explicit operator actions. Onl
y the init / standalone OAP performs the reshape; non-init peers continue
through the existing poll-and-wait loop so a single node drives DDL. When a
`check*` records `SKIPPED_SHAPE_MISMATCH` the dependent `IndexRule` /
`IndexRuleBinding` reconciliation is also skipped — preventing the previous gap
where the binding silently updated to a tag list that diverged from the live
tag-family layout. `AlarmRecord` is opted in. Default remains `false` for all
other models — boot-time reshape stays off unless the annotation is explicitly
set. **Operator caveat:** BanyanDB does not physically migrate existing rows
when a tag's family changes; pre-existing data stays in its original on-disk
location while new writes go to the declared family — expect a backfill window
for queries that route through new IndexRules on relocated tags.
+* Mask keywords `trustStorePass`, `keyStorePass` by default.
Review Comment:
This changelog entry implies the defaults mask
`trustStorePass`/`keyStorePass`, but the status module's code default
(`StatusModuleConfig.keywords4MaskingSecretsOfConfig`) still doesn't include
these keywords. Either update the module config default to match the new
behavior everywhere, or clarify in the changelog that this is only the shipped
`application.yml` default.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
