RHOBS-1583: Bump rhobs/obs-mcp to v0.3.0#329
Open
slashpai wants to merge 3 commits into
Open
Conversation
Signed-off-by: Jayapriya Pai <janantha@redhat.com>
Signed-off-by: Jayapriya Pai <janantha@redhat.com>
Signed-off-by: Jayapriya Pai <janantha@redhat.com>
📝 WalkthroughWalkthroughAdds otelcol toolset docs and cross-links, introduces multiple MCPChecker eval tasks for otelcol, bumps obs-mcp to v0.3.0, and vendors major go-openapi/cbor/runtime updates. Embeds OpenTelemetry Collector config schemas (v0.144.0) and extensive exporter/connector docs and JSON Schemas. ChangesOTel tooling and vendor update
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: slashpai The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Member
Author
openshift-ci-robot
added
the
jira/valid-reference
|
@slashpai: This pull request references RHOBS-1583 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@slashpai: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Member
Author
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
vendor/github.com/go-openapi/jsonpointer/pointer.go (1)
545-627:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix
Pointer.Set(..., nil)insetSingleImplto avoid panics and inconsistent map semantics.
setSingleImplreflectsdatawithreflect.ValueOf(data).Type()in thereflect.Structandreflect.Slicebranches without special-casing untypednil, soPointer.Set(..., nil)can panic. In thereflect.Mapbranch it callsSetMapIndex(kv, reflect.ValueOf(data)), wherereflect.ValueOf(nil)is an invalidreflect.Valuethat causes the map entry to be deleted rather than assigning a nil/zero value. Special-casedata == niland assignreflect.Zero(...)for the destination type(s) instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/go-openapi/jsonpointer/pointer.go` around lines 545 - 627, The setSingleImpl function must handle data == nil safely: for struct branch (involving nm, fld, assignedType) and slice branch (elem, assignedType/elemType) replace reflect.ValueOf(data) with reflect.Zero(assignedType) or reflect.Zero(elemType) when data == nil instead of calling reflect.ValueOf(nil); for the map branch use the map's value type (rValue.Type().Elem()) and set SetMapIndex(kv, reflect.Zero(mapValueType)) when data == nil so you don't pass an invalid reflect.Value (which deletes the key) and to avoid panics from reflect.ValueOf(nil). Ensure all places that currently call reflect.ValueOf(data) first check data == nil and substitute the correct reflect.Zero(...) for assignedType/elemType/map value type before calling fld.Set, rValue.SetMapIndex, reflect.Append, or element.Set.
🟡 Minor comments (18)
vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_debug.md-2-12 (1)
2-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd required blank lines around the status table.
Markdownlint
MD058is triggered because the table is not surrounded by blank lines.Proposed fix
<!-- status autogenerated section --> # Debug Exporter + | Status | | | ------------- |-----------| | Stability | [development]: profiles | | | [alpha]: traces, metrics, logs | | Distributions | [core], [contrib], [k8s] | | Warnings | [Unstable Output Format](`#warnings`) | | Issues | [](https://github.com/open-telemetry/opentelemetry-collector/issues?q=is%3Aopen+is%3Aissue+label%3Aexporter%2Fdebug) [](https://github.com/open-telemetry/opentelemetry-collector/issues?q=is%3Aclosed+is%3Aissue+label%3Aexporter%2Fdebug) | | [Code Owners](https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/CONTRIBUTING.md#becoming-a-code-owner) | [`@andrzej-stencel`](https://www.github.com/andrzej-stencel) | + [development]: https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/component-stability.md#development [alpha]: https://github.com/open-telemetry/opentelemetry-collector/blob/main/docs/component-stability.md#alpha🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_debug.md` around lines 2 - 12, The Status table under the "Debug Exporter" heading is not surrounded by blank lines which triggers markdownlint MD058; fix it by adding a single blank line before the table (between the "Debug Exporter" heading and the line starting with "| Status") and a single blank line after the table (after the last table row) so the table is properly separated from surrounding content.vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlphttp.md-40-40 (1)
40-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix signal name mismatch for
profiles_endpoint.Line 40 says
endpointis ignored “for logs”, but this bullet is forprofiles_endpoint; it should say “for profiles”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlphttp.md` at line 40, The documentation text for the profiles_endpoint bullet uses the wrong signal name — update the sentence that currently reads "If this setting is present the `endpoint` setting is ignored for logs." to instead reference profiles (e.g., "If this setting is present the `endpoint` setting is ignored for profiles.") so the note matches the `profiles_endpoint` setting; edit the `profiles_endpoint` bullet in the markdown to replace "logs" with "profiles".vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_spanmetrics.yaml-27-31 (1)
27-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove duplicate default dimension entry in description.
Line 30 repeats
span.kind; this list should contain unique default dimensions to avoid ambiguity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_spanmetrics.yaml` around lines 27 - 31, The description for the Dimensions field repeats "span.kind" twice; update the description of Dimensions to list unique default dimensions by removing the duplicate "span.kind" entry so the list reads: service.name, span.kind, status.code (and any other intended unique items) in the Dimensions description block.vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_awscloudwatchlogs.md-50-50 (1)
50-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix typo in
external_iddescription.Line 50 has a spelling error:
identitier→identifier.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_awscloudwatchlogs.md` at line 50, The description for the `external_id` field contains a typo ("identitier"); update the text for `external_id` to replace "identitier" with "identifier" so the documentation reads "Shared identifier used when assuming an IAM role in an external AWS account..." and keep the existing AWS IAM Guide link intact.vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_awsxray.md-95-95 (1)
95-95:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winClean up markdown lint issues in code fence and link references
Line [95] should declare a fenced code language, and Line [107]-Line [109] contain duplicate/unused link definitions.
Suggested fix
-``` +```bash export OTEL_RESOURCE_ATTRIBUTES="aws.log.group.names=log-group1&log-group2&log-group3"@@
-[beta]:https://github.com/open-telemetry/opentelemetry-collector#beta
-[contrib]:https://github.com/open-telemetry/opentelemetry-collector-releases/tree/main/distributions/otelcol-contrib
-[AWS]:https://aws-otel.github.io/docs/getting-started/x-ray#configuring-the-aws-x-ray-exporter</details> Also applies to: 107-109 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_awsxray.md
at line 95, The markdown has lint issues: the code fence at the example block
lacks a language tag and the link reference block contains duplicate/unused
definitions; edit the fenced block (around the example export
OTEL_RESOURCE_ATTRIBUTES) to declare a language (e.g., bash) and remove or
consolidate the duplicate link references ([beta], [contrib], [AWS]) at the
bottom so only the required unique links remain, ensuring link labels used in
the document match the remaining definitions.</details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_count.md-257-269 (1)</summary><blockquote> `257-269`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix invalid YAML in the metrics/datapoints conditions example** Line [259], Line [262], Line [266], and Line [269] have unterminated strings, so this example won’t parse as YAML. <details> <summary>Suggested fix</summary> ```diff metrics: my.prod.metric.count: conditions: - - `attributes["env"] == "prod" + - 'attributes["env"] == "prod"' my.test.metric.count: conditions: - - `attributes["env"] == "test" + - 'attributes["env"] == "test"' datapoints: my.prod.datapoint.count: conditions: - - `attributes["env"] == "prod" + - 'attributes["env"] == "prod"' my.test.datapoint.count: conditions: - - `attributes["env"] == "test" + - 'attributes["env"] == "test"'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_count.md` around lines 257 - 269, The conditions lines for my.prod.metric.count, my.test.metric.count, my.prod.datapoint.count and my.test.datapoint.count contain unterminated inline code/strings; fix them by closing the backtick at the end of each condition so each entry becomes `attributes["env"] == "prod"` or `attributes["env"] == "test"`, ensuring the conditions field values are valid YAML strings for the conditions entries.vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_memory_limiter.md-23-39 (1)
23-39:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSpecify the fenced code block language.
The example block is missing a language identifier, which triggers MD040 and reduces syntax highlighting quality.
Suggested fix
-``` +```yaml receivers: otlp: protocols: grpc: @@ spike_limit_percentage: 0.05</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_memory_limiter.md
around lines 23 - 39, The fenced code block in the markdown example for the
memory_limiter extension is missing a language identifier; update the
triple-backtick fence to include "yaml" so syntax highlighting and MD040 linting
pass (locate the example block that shows receivers: otlp: protocols: grpc:
middlewares: - id: memory_limiter ... and changetoyaml).</details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_health_check.md-8-10 (1)</summary><blockquote> `8-10`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Remove the blank line between blockquote sections to satisfy markdownlint.** There is a blank line inside consecutive blockquotes; this triggers MD028 in strict markdown lint setups. <details> <summary>Suggested fix</summary> ```diff > section for details about feature gates and migration options. - > ⚠️⚠️⚠️ **Warning** ⚠️⚠️⚠️ ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_health_check.md` around lines 8 - 10, Remove the extra blank line between consecutive blockquote lines so the two blockquote markers (the lines starting with "> ⚠️⚠️⚠️ **Warning** ⚠️⚠️⚠️" and the following ">" line) are adjacent, eliminating the empty line that triggers markdownlint rule MD028; edit the section containing those blockquote markers to join them without an intervening blank line. ``` </details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_prometheusremotewrite.md-169-171 (1)</summary><blockquote> `169-171`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Remove duplicate link reference definitions.** Lines 169-171 duplicate the `[beta]`, `[contrib]`, and `[core]` definitions already declared at Lines 12-14, which triggers MD053. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_prometheusremotewrite.md` around lines 169 - 171, Remove the duplicate link reference definitions for [beta], [contrib], and [core] (the repeated block that re-declares those labels) so only the original declarations remain; locate the duplicated `[beta]:`, `[contrib]:`, `[core]:` reference lines in the exporter_prometheusremotewrite.md and delete that redundant block (or consolidate it into the initial definitions) to resolve the MD053 duplicate-link-reference issue. ``` </details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_spanmetrics.md-39-39 (1)</summary><blockquote> `39-39`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add languages to fenced code blocks to satisfy markdown lint.** The fences at Line 39, Line 45, Line 52, Line 248, and Line 279 are missing language identifiers (MD040). Also applies to: 45-45, 52-52, 248-248, 279-279 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_spanmetrics.md
at line 39, Several fenced code blocks are missing language identifiers; update
each backticked fence (theblocks around the examples near the sections where example configs are shown) by adding the appropriate language tag (e.g.,yaml,json, orbash/console) so Markdown lint rule MD040 is satisfied;
locate the triple-backtick fences in the connector_spanmetrics.md examples (the
fences currently shown as ``` at the offending spots) and replace them with
language-specific fences matching the content of each block.</details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_spanmetrics.md-133-135 (1)</summary><blockquote> `133-135`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Clean up corrupted sentence in exemplars description.** Line 134 contains a broken fragment (`rom the cache...`) appended to the sentence, which looks like accidental paste/merge noise. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_spanmetrics.md` around lines 133 - 135, The `exemplars` section has a corrupted fragment in the `enabled` description; remove the stray "rom the cache, its next data point will indicate a "reset"..." text and ensure the `enabled` field reads clearly (e.g., "enabled (default: false): enabling will add spans as Exemplars to all metrics. Exemplars are only kept for one flush interval.") while leaving the `max_per_data_point` line unchanged; update the description text associated with `exemplars.enabled` and `exemplars.max_per_data_point` in the same markdown block to reflect this cleanup. ``` </details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlp_http.yaml-108-110 (1)</summary><blockquote> `108-110`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Correct the default `profiles_endpoint` path text.** Line 109 says `"/v1development/profiles"`; this appears to be a typo and should be `"/v1/profiles"` in the description. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlp_http.yaml` around lines 108 - 110, Fix the typo in the profiles_endpoint description: update the text for the profiles_endpoint field (symbol: profiles_endpoint) so the default path reads "/v1/profiles" instead of the incorrect "/v1development/profiles"; keep the rest of the description intact (i.e., "The URL to send profiles to. If omitted the Endpoint + \"/v1/profiles\" will be used."). ``` </details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_spanmetrics.md-45-47 (1)</summary><blockquote> `45-47`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix malformed metric example string.** Line 46 is missing a closing quote after `get_shipping/{shippingId}` which makes the example invalid. <details> <summary>Suggested fix</summary> ```diff -traces.span.metrics.calls{service.name="shipping",span.name="get_shipping/{shippingId},span.kind="SERVER",status.code="Error"} +traces.span.metrics.calls{service.name="shipping",span.name="get_shipping/{shippingId}",span.kind="SERVER",status.code="Error"} ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_spanmetrics.md` around lines 45 - 47, The example metric string for traces.span.metrics.calls is malformed: the span.name value is missing a closing quote. Update the example so span.name is quoted properly (e.g., change span.name="get_shipping/{shippingId} to span.name="get_shipping/{shippingId}") so the full example reads correctly with balanced quotes and commas. ``` </details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_prometheusremotewrite.md-117-125 (1)</summary><blockquote> `117-125`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix mismatched feature-gate guidance.** Line 121 documents enabling `exporter.prometheusremotewritexporter.RetryOn429` but the example command enables `telemetry.useOtelForInternalMetrics`. Line 125 also includes an extra `+` in the gate name, which makes the literal incorrect. <details> <summary>Suggested doc fix</summary> ```diff -This exporter has feature gate: `exporter.prometheusremotewritexporter.RetryOn429`. +This exporter has feature gate: `exporter.prometheusremotewritexporter.RetryOn429`. -To enable it run collector with enabled feature gate `exporter.prometheusremotewritexporter.RetryOn429`. This can be done by executing it with one additional parameter - `--feature-gates=telemetry.useOtelForInternalMetrics`. +To enable it run collector with enabled feature gate `exporter.prometheusremotewritexporter.RetryOn429`. This can be done by executing it with one additional parameter - `--feature-gates=exporter.prometheusremotewritexporter.RetryOn429`. -This exporter has feature gate: `+exporter.prometheusremotewritexporter.EnableMultipleWorkers`. +This exporter has feature gate: `exporter.prometheusremotewritexporter.EnableMultipleWorkers`. ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_prometheusremotewrite.md` around lines 117 - 125, The doc text incorrectly instructs enabling exporter.prometheusremotewritexporter.RetryOn429 by showing the unrelated flag telemetry.useOtelForInternalMetrics and also shows a malformed gate name +exporter.prometheusremotewritexporter.EnableMultipleWorkers; update the example command to use the correct feature-gates entry for RetryOn429 (e.g. --feature-gates=exporter.prometheusremotewritexporter.RetryOn429) and remove the stray plus sign from EnableMultipleWorkers so it reads exporter.prometheusremotewritexporter.EnableMultipleWorkers. ``` </details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_loadbalancing.md-56-74 (1)</summary><blockquote> `56-74`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add a language to the fenced code block.** The diagram block is missing a language token, which triggers MD040 in markdownlint. <details> <summary>Suggested fix</summary> ```diff -``` +```text ... -``` +``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_loadbalancing.md
around lines 56 - 74, The fenced diagram block (the triple backticks surrounding
the ASCII diagram) is missing a language token; update the opening fence from
totext so markdownlint MD040 is satisfied and the diagram is treated as
plain text (i.e., change thethat precedes the ASCII art totext while
leaving the closing ``` unchanged).</details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_loadbalancing.md-224-224 (1)</summary><blockquote> `224-224`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix extension ID in the example service block.** `service.extensions` should reference `file_storage/otc` to match the declared extension and queue storage target; current value makes the example invalid. <details> <summary>Suggested fix</summary> ```diff -service: - extensions: [file_storage] +service: + extensions: [file_storage/otc] ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_loadbalancing.md` at line 224, The example service block references the wrong extension ID; update the service.extensions entry to match the declared extension and queue storage target by replacing the current value with "file_storage/otc" so service.extensions points to the same extension ID used in the configuration (ensure you update the service.extensions array in the example to include "file_storage/otc"). ``` </details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_file_storage.md-35-35 (1)</summary><blockquote> `35-35`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Fix reversed Markdown link syntax on Line 35.** The current link is reversed and won’t render as a clickable issue reference. <details> <summary>Proposed fix</summary> ```diff -See (`#35899`)[https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/35899] for more details. +See [`#35899`](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/35899) for more details. ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_file_storage.md` at line 35, Replace the reversed Markdown link text "See (`#35899`)[https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/35899]" with proper Markdown link syntax by swapping the label and URL into "[`#35899`](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/35899)" so the issue reference renders as a clickable link. ``` </details> </blockquote></details> <details> <summary>vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_file_storage.md-66-66 (1)</summary><blockquote> `66-66`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_ **Add a language tag to the fenced block at Line 66.** This avoids MD040 failures in markdown linting. <details> <summary>Proposed fix</summary> ```diff -``` +```text ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_file_storage.md
at line 66, The fenced code block at Line 66 is missing a language tag which
triggers MD040; update the fenced block in extension_file_storage.md so the
opening backticks include a language tag (e.g., change "" to "text") to
satisfy the linter and preserve the block content.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (1)</summary><blockquote> <details> <summary>vendor/github.com/go-openapi/runtime/middleware/context.go (1)</summary><blockquote> `692-699`: _⚡ Quick win_ **Make `respondWithResponder` resilient only if `route` can be nil when `data` is a `Responder`.** `respondWithResponder` dereferences `route.Producers` without a nil guard; this can panic if `Context.Respond` is called with a `Responder` and `route == nil`. In the go-openapi runtime middleware flows, `route == nil` appears to be used for error responses (handled by `respondWithError`, which already guards `route`), so this nil panic path may be unreachable in normal usage. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@vendor/github.com/go-openapi/runtime/middleware/context.go` around lines 692 - 699, The function respondWithResponder dereferences route.Producers without checking route for nil; add a guard at the top of respondWithResponder to handle the case route == nil: if route is nil, skip the MIME negotiation (skip using normalizeOffer and route.Producers) and directly invoke the Responder's write path with a sensible default producer (e.g., JSON/text default from your runtime), otherwise continue existing logic (use producers := route.Producers, prod lookup via normalizeOffer(format) etc.); ensure you reference respondWithResponder, route.Producers, normalizeOffer(format) and Responder.WriteResponse (or the resp's write method) in the change so the nil-path does not panic. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.Inline comments:
In@vendor/github.com/fxamacker/cbor/v2/decode.go:
- Around line 2764-2801: parseMapToStruct currently calls handleUnmatchedMapKey
with string(keyBytes) for both text and byte-string keys, which normalizes
byte-string keys into strings and breaks duplicate-key tracking and type
semantics; change parseMapToStruct so it preserves the original key
representation when calling handleUnmatchedMapKey (i.e., pass the []byte
keyBytes directly when keyType == cborTypeByteString and only convert to string
for cborTypeTextString), and update handleUnmatchedMapKey (and its internal
duplicate-tracking) to accept the raw key plus keyType (or an interface{}
rawKey) and deduplicate using type-aware comparison (use bytes.Equal for []byte
keys and normal map indexing or string equality for text keys) instead of
relying on map[any]struct{} with forced string normalization; refer to
parseMapToStruct, keyType, keyBytes, handleUnmatchedMapKey and DupMapKeyMode in
your changes.In
@vendor/github.com/go-openapi/runtime/client_response.go:
- Line 62: In APIError.Error() replace the ignored json.Marshal call so its
error is checked: call json.Marshal(o.Response) capturing (resp, err), and if
err != nil include that error text in the returned error message (or use a clear
fallback message that embeds err.Error()); ensure resp is only used when marshal
succeeds and that o.Response is still referenced in the constructed string when
appropriate. This updates the marshalling of o.Response to surface marshal
failures instead of swallowing them.In
@vendor/github.com/go-openapi/runtime/middleware/context.go:
- Around line 769-776: The lookups against route.Producers in respondWithCode
(and the similar lookup in respondWithoutCode) use the raw format string which
may contain media-type parameters (e.g. "text/plain; charset=utf-8") and thus
miss keys that are stored as bare media types; before using producers[format]
normalize the negotiated format to the bare media type (strip any "; ..."
params) — for example call the same normalization used elsewhere (e.g.
normalizeOffers or use mime.ParseMediaType) to produce a key like "text/plain"
and then use that normalized value for the lookup and any fallback logic that
calls c.api.ProducersFor or cantFindProducer.In
@vendor/github.com/go-openapi/runtime/middleware/router.go:
- Around line 352-357: defaultRouter.Lookup currently panics when len(d.routers)
== 0 which converts a legitimate empty-router case (e.g. when
defaultRouteBuilder.Build() produces no routes) into a request-time crash;
change the Lookup behavior to not panic on an empty d.routers but instead treat
it as a normal miss—keep the Debug debugLogf call if desired, remove the panic,
and return the appropriate "not found" result (so callers receive a nil/no-match
result and existing NotFound/MethodNotAllowed handling can proceed). Ensure you
modify the logic in defaultRouter.Lookup (and any closely related return path
expectations) rather than changing defaultRouteBuilder.Build.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_routing.yaml:
- Around line 25-29: The schema documents properties.table as required but
doesn't enforce it; update the root object in connector_routing.yaml to include
a top-level required array that lists "table" (i.e., add required: ["table"]
alongside the existing properties definition) so that the JSON schema fails
validation when the table property is omitted.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_awscloudwatchlogs.yaml:
- Around line 103-108: The schema for the sizer fields is incorrect: update the
sending_queue.batch.sizer and sending_queue.sizer definitions (symbols:
sending_queue.batch.sizer, sending_queue.sizer, sizer) so that the type reflects
the documented allowed values (change from type: object to type: string and
constrain values to an enum of "requests", "items", "bytes"); ensure the
description remains, add an enum array with those three strings and remove
object-specific properties if present so the schema contract matches the
documentation.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_debug.yaml:
- Around line 28-33: The "sizer" schema currently declares type: object but its
description and allowed values are scalar strings ("requests", "items",
"bytes"), causing valid configs to be rejected; update the "sizer" schema
entries (the sizer fields in exporter_debug.yaml) to use type: string and add an
enum with the allowed values ["requests","items","bytes"] so the schema accepts
and validates the documented scalar values.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_googlecloud.yaml:
- Around line 182-188: The schema wrongly declares sending_queue.batch.sizer and
sending_queue.sizer as type: object while the description and accepted values
are scalar; update both definitions to use type: string and add an enum with the
allowed values "requests", "items", and "bytes" (i.e., replace type: object with
type: string + enum) so valid scalar configs are accepted during validation.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_kafka.yaml:
- Around line 452-458: The sizer property is declared with type: object but its
description and valid values indicate it should be a string enum; update both
sizer schema blocks (the ones currently using type: object) to type: string and
add an enum of ["requests","items","bytes"] (remove object-specific constraints
like additionalProperties if present) so valid configurations using those string
choices are accepted — refer to the sizer schema entries to locate both
occurrences to change.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlp_http.md:
- Around line 39-40: The documentation for profiles_endpoint contains an
incorrect example path and wrong signal reference; update the description for
profiles_endpoint to use the correct path "/v1/profiles" instead of
"/v1development/profiles" and change the sentence that currently says the
endpointsetting is ignored "for logs" to state it is ignored "for profiles";
locate this in the exporter_otlp_http.md entry forprofiles_endpointand
adjust the example URL and the final clause accordingly.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlp_http.yaml:
- Around line 167-173: The schema declares the "sizer" field as type: object but
the description says it should accept the string values "requests", "items", or
"bytes"; change the "sizer" schema (both occurrences labeled sizer) to type:
string and add an enum with the allowed values ["requests","items","bytes"] (or
otherwise constrain it to string values) so the validator enforces the
documented options instead of expecting an object.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlp.yaml:
- Around line 113-118: The schema for the sizer entries is incorrect: change the
type for both sending_queue.batch.sizer and sending_queue.sizer from "object" to
"string" and add an enum with the allowed values "requests", "items", and
"bytes" so validation accepts those string enums; locate the sizer blocks
(symbols: sending_queue.batch.sizer and sending_queue.sizer) in the YAML and
update their type and enum accordingly.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlphttp.yaml:
- Around line 167-172: The schema declares the "sizer" field as type: object but
its description and allowed values indicate it should be a string enum; update
both occurrences of the "sizer" schema to use type: string and add an enum:
["requests", "items", "bytes"] (and keep the existing description) so the YAML
matches the documented config shape for the sizer field.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_bearertokenauth.yaml:
- Around line 17-19: The schema declares the tokens field with the wrong type:
change the "tokens" property in extension_bearertokenauth.yaml from type: string
to an array of strings by setting type: array and adding items: { type: string }
so it matches the upstream bearertokenauthextension contract; update the
"tokens" mapping (the tokens property) accordingly to validate YAML lists like
tokens: ["token1","token2"].
Outside diff comments:
In@vendor/github.com/go-openapi/jsonpointer/pointer.go:
- Around line 545-627: The setSingleImpl function must handle data == nil
safely: for struct branch (involving nm, fld, assignedType) and slice branch
(elem, assignedType/elemType) replace reflect.ValueOf(data) with
reflect.Zero(assignedType) or reflect.Zero(elemType) when data == nil instead of
calling reflect.ValueOf(nil); for the map branch use the map's value type
(rValue.Type().Elem()) and set SetMapIndex(kv, reflect.Zero(mapValueType)) when
data == nil so you don't pass an invalid reflect.Value (which deletes the key)
and to avoid panics from reflect.ValueOf(nil). Ensure all places that currently
call reflect.ValueOf(data) first check data == nil and substitute the correct
reflect.Zero(...) for assignedType/elemType/map value type before calling
fld.Set, rValue.SetMapIndex, reflect.Append, or element.Set.
Minor comments:
In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_count.md:
- Around line 257-269: The conditions lines for my.prod.metric.count,
my.test.metric.count, my.prod.datapoint.count and my.test.datapoint.count
contain unterminated inline code/strings; fix them by closing the backtick at
the end of each condition so each entry becomesattributes["env"] == "prod"or
attributes["env"] == "test", ensuring the conditions field values are valid
YAML strings for the conditions entries.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_spanmetrics.md:
- Line 39: Several fenced code blocks are missing language identifiers; update
each backticked fence (theblocks around the examples near the sections where example configs are shown) by adding the appropriate language tag (e.g.,yaml,json, orbash/console) so Markdown lint rule MD040 is satisfied;
locate the triple-backtick fences in the connector_spanmetrics.md examples (the
fences currently shown as ``` at the offending spots) and replace them with
language-specific fences matching the content of each block.- Around line 133-135: The
exemplarssection has a corrupted fragment in the
enableddescription; remove the stray "rom the cache, its next data point will
indicate a "reset"..." text and ensure theenabledfield reads clearly (e.g.,
"enabled (default: false): enabling will add spans as Exemplars to all metrics.
Exemplars are only kept for one flush interval.") while leaving the
max_per_data_pointline unchanged; update the description text associated with
exemplars.enabledandexemplars.max_per_data_pointin the same markdown
block to reflect this cleanup.- Around line 45-47: The example metric string for traces.span.metrics.calls is
malformed: the span.name value is missing a closing quote. Update the example so
span.name is quoted properly (e.g., change span.name="get_shipping/{shippingId}
to span.name="get_shipping/{shippingId}") so the full example reads correctly
with balanced quotes and commas.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_spanmetrics.yaml:
- Around line 27-31: The description for the Dimensions field repeats
"span.kind" twice; update the description of Dimensions to list unique default
dimensions by removing the duplicate "span.kind" entry so the list reads:
service.name, span.kind, status.code (and any other intended unique items) in
the Dimensions description block.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_awscloudwatchlogs.md:
- Line 50: The description for the
external_idfield contains a typo
("identitier"); update the text forexternal_idto replace "identitier" with
"identifier" so the documentation reads "Shared identifier used when assuming an
IAM role in an external AWS account..." and keep the existing AWS IAM Guide link
intact.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_awsxray.md:
- Line 95: The markdown has lint issues: the code fence at the example block
lacks a language tag and the link reference block contains duplicate/unused
definitions; edit the fenced block (around the example export
OTEL_RESOURCE_ATTRIBUTES) to declare a language (e.g., bash) and remove or
consolidate the duplicate link references ([beta], [contrib], [AWS]) at the
bottom so only the required unique links remain, ensuring link labels used in
the document match the remaining definitions.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_debug.md:
- Around line 2-12: The Status table under the "Debug Exporter" heading is not
surrounded by blank lines which triggers markdownlint MD058; fix it by adding a
single blank line before the table (between the "Debug Exporter" heading and the
line starting with "| Status") and a single blank line after the table (after
the last table row) so the table is properly separated from surrounding content.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_loadbalancing.md:
- Around line 56-74: The fenced diagram block (the triple backticks surrounding
the ASCII diagram) is missing a language token; update the opening fence from
totext so markdownlint MD040 is satisfied and the diagram is treated as
plain text (i.e., change thethat precedes the ASCII art totext while
leaving the closing ``` unchanged).- Line 224: The example service block references the wrong extension ID; update
the service.extensions entry to match the declared extension and queue storage
target by replacing the current value with "file_storage/otc" so
service.extensions points to the same extension ID used in the configuration
(ensure you update the service.extensions array in the example to include
"file_storage/otc").In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlp_http.yaml:
- Around line 108-110: Fix the typo in the profiles_endpoint description: update
the text for the profiles_endpoint field (symbol: profiles_endpoint) so the
default path reads "/v1/profiles" instead of the incorrect
"/v1development/profiles"; keep the rest of the description intact (i.e., "The
URL to send profiles to. If omitted the Endpoint + "/v1/profiles" will be
used.").In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlphttp.md:
- Line 40: The documentation text for the profiles_endpoint bullet uses the
wrong signal name — update the sentence that currently reads "If this setting is
present theendpointsetting is ignored for logs." to instead reference
profiles (e.g., "If this setting is present theendpointsetting is ignored
for profiles.") so the note matches theprofiles_endpointsetting; edit the
profiles_endpointbullet in the markdown to replace "logs" with "profiles".In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_prometheusremotewrite.md:
- Around line 169-171: Remove the duplicate link reference definitions for
[beta], [contrib], and [core] (the repeated block that re-declares those labels)
so only the original declarations remain; locate the duplicated[beta]:,
[contrib]:,[core]:reference lines in the exporter_prometheusremotewrite.md
and delete that redundant block (or consolidate it into the initial definitions)
to resolve the MD053 duplicate-link-reference issue.- Around line 117-125: The doc text incorrectly instructs enabling
exporter.prometheusremotewritexporter.RetryOn429 by showing the unrelated flag
telemetry.useOtelForInternalMetrics and also shows a malformed gate name
+exporter.prometheusremotewritexporter.EnableMultipleWorkers; update the example
command to use the correct feature-gates entry for RetryOn429 (e.g.
--feature-gates=exporter.prometheusremotewritexporter.RetryOn429) and remove the
stray plus sign from EnableMultipleWorkers so it reads
exporter.prometheusremotewritexporter.EnableMultipleWorkers.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_file_storage.md:
- Line 35: Replace the reversed Markdown link text "See
(#35899)[https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/35899]"
with proper Markdown link syntax by swapping the label and URL into
"#35899"
so the issue reference renders as a clickable link.- Line 66: The fenced code block at Line 66 is missing a language tag which
triggers MD040; update the fenced block in extension_file_storage.md so the
opening backticks include a language tag (e.g., change "" to "text") to
satisfy the linter and preserve the block content.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_health_check.md:
- Around line 8-10: Remove the extra blank line between consecutive blockquote
lines so the two blockquote markers (the lines starting with ">⚠️ ⚠️ ⚠️
Warning⚠️ ⚠️ ⚠️ " and the following ">" line) are adjacent, eliminating the
empty line that triggers markdownlint rule MD028; edit the section containing
those blockquote markers to join them without an intervening blank line.In
@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_memory_limiter.md:
- Around line 23-39: The fenced code block in the markdown example for the
memory_limiter extension is missing a language identifier; update the
triple-backtick fence to include "yaml" so syntax highlighting and MD040 linting
pass (locate the example block that shows receivers: otlp: protocols: grpc:
middlewares: - id: memory_limiter ... and changetoyaml).
Nitpick comments:
In@vendor/github.com/go-openapi/runtime/middleware/context.go:
- Around line 692-699: The function respondWithResponder dereferences
route.Producers without checking route for nil; add a guard at the top of
respondWithResponder to handle the case route == nil: if route is nil, skip the
MIME negotiation (skip using normalizeOffer and route.Producers) and directly
invoke the Responder's write path with a sensible default producer (e.g.,
JSON/text default from your runtime), otherwise continue existing logic (use
producers := route.Producers, prod lookup via normalizeOffer(format) etc.);
ensure you reference respondWithResponder, route.Producers,
normalizeOffer(format) and Responder.WriteResponse (or the resp's write method)
in the change so the nil-path does not panic.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Comment on lines
+2764
to
+2801
| switch keyType { | ||
| case cborTypeTextString, cborTypeByteString: | ||
| var keyBytes []byte | ||
| if t == cborTypeTextString { | ||
| keyBytes, lastErr = d.parseTextString() | ||
| if lastErr != nil { | ||
| var parseErr error | ||
| keyBytes, parseErr = d.parseTextString() | ||
| if parseErr != nil { | ||
| if err == nil { | ||
| err = lastErr | ||
| err = parseErr | ||
| } | ||
| d.skip() // skip value | ||
| d.skip() // Skip value | ||
| continue | ||
| } | ||
| } else { // cborTypeByteString | ||
| keyBytes, _ = d.parseByteString() | ||
| } | ||
|
|
||
| // Check for exact match on field name. | ||
| if i, ok := structType.fieldIndicesByName[string(keyBytes)]; ok { | ||
| fld := structType.fields[i] | ||
| // Find matching struct field (exact match, then case-insensitive fallback). | ||
| if fldIdx, ok := findStructFieldByKey(structType, keyBytes, caseInsensitive); ok { | ||
| fld := structType.fields[fldIdx] | ||
|
|
||
| if !foundFldIdx[i] { | ||
| f = fld | ||
| foundFldIdx[i] = true | ||
| } else if d.dm.dupMapKey == DupMapKeyEnforcedAPF { | ||
| err = &DupMapKeyError{fld.name, j} | ||
| d.skip() // skip value | ||
| j++ | ||
| // skip the rest of the map | ||
| for ; (hasSize && j < count) || (!hasSize && !d.foundBreak()); j++ { | ||
| d.skip() | ||
| d.skip() | ||
| switch checkDupField(d.dm, foundFldIdx, fldIdx) { | ||
| case mapActionParseValueAndContinue: | ||
| if fieldErr := d.decodeToStructField(v, fld, tInfo); fieldErr != nil && err == nil { | ||
| err = fieldErr | ||
| } | ||
| return err | ||
| } else { | ||
| // discard repeated match | ||
| continue | ||
| case mapActionSkipAllAndReturnError: | ||
| return d.skipMapForDupKey(string(keyBytes), i, count, hasSize) | ||
| case mapActionSkipValueAndContinue: | ||
| d.skip() | ||
| continue MapEntryLoop | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| // Find field with case-insensitive match | ||
| if f == nil && d.dm.fieldNameMatching == FieldNameMatchingPreferCaseSensitive { | ||
| keyLen := len(keyBytes) | ||
| keyString := string(keyBytes) | ||
| for i := 0; i < len(structType.fields); i++ { | ||
| fld := structType.fields[i] | ||
| if len(fld.name) == keyLen && strings.EqualFold(fld.name, keyString) { | ||
| if !foundFldIdx[i] { | ||
| f = fld | ||
| foundFldIdx[i] = true | ||
| } else if d.dm.dupMapKey == DupMapKeyEnforcedAPF { | ||
| err = &DupMapKeyError{keyString, j} | ||
| d.skip() // skip value | ||
| j++ | ||
| // skip the rest of the map | ||
| for ; (hasSize && j < count) || (!hasSize && !d.foundBreak()); j++ { | ||
| d.skip() | ||
| d.skip() | ||
| } | ||
| return err | ||
| } else { | ||
| // discard repeated match | ||
| d.skip() | ||
| continue MapEntryLoop | ||
| } | ||
| break | ||
| } | ||
| } | ||
| // No matching struct field found. | ||
| if unmatchedErr := handleUnmatchedMapKey(d, string(keyBytes), i, count, hasSize, &unmatchedMapKeys); unmatchedErr != nil { | ||
| return unmatchedErr |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="vendor/github.com/fxamacker/cbor/v2/decode.go"
echo "== Context around lines 2764-2801 =="
sed -n '2745,2820p' "$FILE" | nl -ba | sed 's/^ *//' || true
echo
echo "== Find DupMapKeyMode docs and related config names =="
rg -n "DupMapKeyMode|DupMapKeyEnforcedAPF|FieldNameByteStringAllowed|handleUnmatchedMapKey|unmatchedMapKeys|checkDupField" "$FILE" || true
echo
echo "== Show handleUnmatchedMapKey implementation =="
rg -n "func handleUnmatchedMapKey" "$FILE" || true
# Print a reasonable region around the function if found
line=$(rg -n "func handleUnmatchedMapKey" "$FILE" | head -n1 | cut -d: -f1 || true)
if [[ -n "${line:-}" ]]; then
start=$((line-60)); end=$((line+220))
sed -n "${start},${end}p" "$FILE" | nl -ba | sed 's/^ *//' || true
fi
echo
echo "== Trace where unmatchedMapKeys are updated/compared =="
# Search for unmatchedMapKeys usage and comparison logic
rg -n "unmatchedMapKeys" "$FILE" || true
echo
echo "== Check how key bytes are converted earlier (string(keyBytes) etc) =="
rg -n "string\\(keyBytes\\)" "$FILE" || true
echo
echo "== Locate ByteString definition / type used for byte-string keys =="
rg -n "\\btype\\s+ByteString\\b" "$FILE" || true
# If not in decode.go, search in vendor package for ByteString type
if ! rg -n "\\btype\\s+ByteString\\b" "$FILE" >/dev/null; then
echo "Searching entire vendor fxamacker/cbor/v2 for ByteString type..."
rg -n "\\btype\\s+ByteString\\b" "vendor/github.com/fxamacker/cbor/v2" || true
fiRepository: openshift/openshift-mcp-server
Length of output: 2795
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="vendor/github.com/fxamacker/cbor/v2/decode.go"
echo "== DupMapKeyMode contract (around line ~300) =="
sed -n '280,370p' "$FILE" | cat -n
echo
echo "== Dup-map tracking key type initialization and surrounding logic (2700-2920) =="
sed -n '2680,2920p' "$FILE" | cat -n
echo
echo "== ByteString definition (bytestring.go) =="
sed -n '1,120p' vendor/github.com/fxamacker/cbor/v2/bytestring.go | cat -n
echo
echo "== handleUnmatchedMapKey implementation =="
line="$(rg -n "func handleUnmatchedMapKey" "$FILE" | head -n1 | cut -d: -f1 || true)"
echo "Found at line: ${line:-<none>}"
if [[ -n "${line:-}" ]]; then
start=$((line-40)); end=$((line+220))
sed -n "${start},${end}p" "$FILE" | cat -n
fi
echo
echo "== Any other uses of handleUnmatchedMapKey =="
rg -n "handleUnmatchedMapKey\(" "$FILE"
echo
echo "== Other conversions to track unmatched keys (check for string(keyBytes) / ByteString(keyBytes)) =="
rg -n "handleUnmatchedMapKey\\(.*string\\(keyBytes\\)|ByteString\\(keyBytes\\)|string\\(keyBytes\\)" "$FILE"Repository: openshift/openshift-mcp-server
Length of output: 16641
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate handleUnmatchedMapKey across vendor fxamacker/cbor/v2 =="
rg -n "func handleUnmatchedMapKey" vendor/github.com/fxamacker/cbor/v2 || true
echo
echo "== If found, show its implementation region =="
FILE="$(rg -l "func handleUnmatchedMapKey" vendor/github.com/fxamacker/cbor/v2 | head -n1 || true)"
if [[ -n "${FILE:-}" ]]; then
echo "Found in: $FILE"
sed -n '1,220p' "$FILE" | cat -n | head -n 220 >/dev/null # just to ensure file exists
line="$(rg -n "func handleUnmatchedMapKey" "$FILE" | head -n1 | cut -d: -f1)"
start=$((line-60)); end=$((line+220))
sed -n "${start},${end}p" "$FILE" | nl -ba | head -n 320
fi
echo
echo "== Search for unmatchedMapKeys usage within the whole package =="
rg -n "unmatchedMapKeys" vendor/github.com/fxamacker/cbor/v2 || true
echo
echo "== Search for how byte-string keys are converted for duplicate tracking =="
rg -n "DupMapKey|dupMapKey|ByteString\\(|\\[\\]byte|isHashableValue" vendor/github.com/fxamacker/cbor/v2/decode.go vendor/github.com/fxamacker/cbor/v2/*.go || trueRepository: openshift/openshift-mcp-server
Length of output: 404
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="vendor/github.com/fxamacker/cbor/v2/decode_map_utils.go"
echo "== Function body: handleUnmatchedMapKey (approx 1k lines around it) =="
line="$(rg -n "func handleUnmatchedMapKey" "$FILE" | head -n1 | cut -d: -f1)"
start=$((line-120)); end=$((line+260))
sed -n "${start},${end}p" "$FILE" | cat -n
echo
echo "== Show any helper used for duplicate checking inside decode_map_utils.go =="
rg -n "dupMapKey|unmatchedMapKeys|skipMapForDupKey|checkDupField|isHashableValue" "$FILE" || true
echo
echo "== Show skipMapForDupKey signature/usage (if nearby) =="
rg -n "skipMapForDupKey" vendor/github.com/fxamacker/cbor/v2/decode_map_utils.go || true
echo
echo "== Show map tracking key construction (search in decode_map_utils.go for map[any]struct{} or map key types) =="
rg -n "map\\[any\\]struct\\{}|map\\[.*\\]struct\\{}|unmatchedMapKeys" "$FILE" || true
echo
echo "== Confirm how ByteString is used in unmatched/dup-map code =="
rg -n "ByteString\\(" vendor/github.com/fxamacker/cbor/v2 || trueRepository: openshift/openshift-mcp-server
Length of output: 1986
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="vendor/github.com/fxamacker/cbor/v2/decode_map_utils.go"
line="$(rg -n "func handleUnmatchedMapKey" "$FILE" | head -n1 | cut -d: -f1)"
echo "handleUnmatchedMapKey at line: $line"
start=$((line-120)); if (( start < 1 )); then start=1; fi
end=$((line+260))
echo
echo "== Source excerpt: ${FILE} (lines ${start}-${end}) =="
sed -n "${start},${end}p" "$FILE" | awk '{printf "%6d\t%s\n", NR+('"$start"'-1), $0}'
echo
echo "== Where unmatchedMapKeys is used/compared in this file =="
rg -n "unmatchedMapKeys|dupMapKey|skipMapForDupKey|ByteString\\(" "$FILE"
echo
echo "== Key normalization / conversion patterns in this file =="
rg -n "string\\(|ByteString\\(|\\[\\]byte|isHashableValue" "$FILE"Repository: openshift/openshift-mcp-server
Length of output: 4714
🏁 Script executed:
#!/bin/bash
set -euo pipefail
BASE="vendor/github.com/fxamacker/cbor/v2"
DECODE="$BASE/decode.go"
echo "== Locate parseByteString implementation =="
rg -n "func \\(d \\*decoder\\) parseByteString\\b" "$DECODE" "$BASE" || true
echo
echo "== Locate MapKeyByteStringMode / related options =="
rg -n "MapKeyByteStringMode|mapKeyByteString|ByteStringMode|fieldNameByteString" "$BASE" || true
echo
echo "== Show parse() byte-string handling (best-effort) =="
# Print some region around where byte strings are dispatched in parse
# Search for "cborTypeByteString" in parse()
line="$(rg -n "cborTypeByteString" "$DECODE" | head -n 20 | tail -n 1 | cut -d: -f1 || true)"
echo "Sample line with cborTypeByteString in decode.go: ${line:-<none>}"
if [[ -n "${line:-}" ]]; then
start=$((line-60)); if (( start < 1 )); then start=1; fi
end=$((line+120))
sed -n "${start},${end}p" "$DECODE" | head -n 200
fi
echo
echo "== Locate any helper isHashableValue in package =="
rg -n "func isHashableValue" "$BASE" || trueRepository: openshift/openshift-mcp-server
Length of output: 7745
🏁 Script executed:
#!/bin/bash
set -euo pipefail
BASE="vendor/github.com/fxamacker/cbor/v2"
DECODE="$BASE/decode.go"
echo "== parseByteString implementation =="
start="$(rg -n "func \\(d \\*decoder\\) parseByteString" "$DECODE" | head -n1 | cut -d: -f1)"
end=$((start+120))
sed -n "${start},${end}p" "$DECODE" | cat -n
echo
echo "== Locate parse() byte-string -> interface{} behavior (MapKeyByteStringMode) =="
# Find parse() and then focus on byte string branch inside it.
parse_line="$(rg -n "func \\(d \\*decoder\\) parse\\(" "$DECODE" | head -n1 | cut -d: -f1)"
echo "parse() starts at line: $parse_line"
# Show a wider region around where it handles byte strings in parse()
# (limit output by showing only the matching case blocks)
rg -n "cborTypeByteString" "$DECODE" | head -n 40
echo
echo "== Exact case blocks for cborTypeByteString inside parse() =="
# Print around each occurrence that’s likely inside parse()
rg -n "case cborTypeByteString" "$DECODE" || true
echo
echo "== isHashableValue implementation =="
is_hash_line="$(rg -n "func isHashableValue\\(" "$DECODE" | head -n1 | cut -d: -f1)"
sed -n "${is_hash_line},$((is_hash_line+120))p" "$DECODE" | cat -n
echo
echo "== decode_map_utils.go skipMapForDupKey uses key (string vs ByteString) =="
# locate skipMapForDupKey in package
rg -n "func \\(d \\*decoder\\) skipMapForDupKey" "$BASE"
skip_line="$(rg -n "func \\(d \\*decoder\\) skipMapForDupKey" "$BASE/decode*.go" | head -n1 | cut -d: -f1 || true)"
if [[ -n "${skip_line:-}" ]]; then
# get file path too
skip_file="$(rg -n "func \\(d \\*decoder\\) skipMapForDupKey" "$BASE" | head -n1 | cut -d: -f1)"
sed -n "1,40p" "$skip_file" | cat -n | head -n 40
sed -n "$((skip_line-20)),$((skip_line+120))p" "$skip_file" | cat -n
fiRepository: openshift/openshift-mcp-server
Length of output: 7746
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="vendor/github.com/fxamacker/cbor/v2/decode.go"
echo "== parse() byte-string case blocks with surrounding code =="
# For each "case cborTypeByteString:" occurrence, print a small window.
for line in $(rg -n "case cborTypeByteString:" "$FILE" | cut -d: -f1 | sort -u); do
start=$((line-40)); if (( start < 1 )); then start=1; fi
end=$((line+120))
echo
echo "---- around case at line $line (lines ${start}-${end}) ----"
sed -n "${start},${end}p" "$FILE" | cat -n
done
echo
echo "== Also show any 'mapKeyByteString' decision logic in parse() =="
rg -n "mapKeyByteString|MapKeyByteStringAllowed|MapKeyByteString|convertByteSliceToByteString" "$FILE" | head -n 80
echo
echo "== Show skipMapForDupKey error handling =="
skip_file="$(rg -n "func \\(d \\*decoder\\) skipMapForDupKey" "$FILE" | cut -d: -f1 | head -n1)"
skip_line="$(rg -n "func \\(d \\*decoder\\) skipMapForDupKey" "$FILE" | head -n1 | cut -d: -f1)"
start=$((skip_line-40)); if (( start < 1 )); then start=1; fi
end=$((skip_line+120))
sed -n "${start},${end}p" "$skip_file" | cat -nRepository: openshift/openshift-mcp-server
Length of output: 19624
🏁 Script executed:
#!/bin/bash
set -euo pipefail
BASE="vendor/github.com/fxamacker/cbor/v2"
DECODE="$BASE/decode.go"
echo "== Find defaultByteStringType definition and assignments =="
rg -n "defaultByteStringType" "$DECODE" "$BASE" || true
echo
echo "== Show decMode / defaultByteStringType initialization block (if any) =="
# try to print around struct decMode definition
line="$(rg -n "type decMode struct" "$DECODE" | head -n1 | cut -d: -f1 || true)"
if [[ -n "${line:-}" ]]; then
sed -n "${line},$((line+220))p" "$DECODE" | cat -n
fiRepository: openshift/openshift-mcp-server
Length of output: 11635
🏁 Script executed:
#!/bin/bash
set -euo pipefail
BASE="vendor/github.com/fxamacker/cbor/v2"
DECODE="$BASE/decode.go"
echo "== Locate DecOptions.DefaultByteStringType docs/defaults =="
rg -n "DefaultByteStringType|defaultByteStringType" "$DECODE" || true
echo
echo "== Show DecOptions struct fields around DefaultByteStringType =="
opts_line="$(rg -n "type DecOptions struct" "$DECODE" | head -n1 | cut -d: -f1)"
start=$((opts_line-20)); if (( start < 1 )); then start=1; fi
end=$((opts_line+200))
sed -n "${start},${end}p" "$DECODE" | cat -n
echo
echo "== Show DecOptions.decMode() where defaultByteStringType set =="
dm_line="$(rg -n "func \\(opts \\*?DecOptions\\) decMode\\(" "$DECODE" | head -n1 | cut -d: -f1 || true)"
if [[ -z "${dm_line:-}" ]]; then
dm_line="$(rg -n "decMode\\(" "$DECODE" | rg -n "DecOptions" | head -n1 | cut -d: -f1 || true)"
fi
echo "decMode func at: ${dm_line:-<none>}"
if [[ -n "${dm_line:-}" ]]; then
start=$((dm_line-40)); if (( start < 1 )); then start=1; fi
end=$((dm_line+260))
sed -n "${start},${end}p" "$DECODE" | cat -n
fi
echo
echo "== Show option MapKeyByteString and its docs =="
rg -n "type DecOptions struct|MapKeyByteString|MapKeyByteStringAllowed|FieldNameByteStringAllowed" "$DECODE" | head -n 80Repository: openshift/openshift-mcp-server
Length of output: 27170
🏁 Script executed:
#!/bin/bash
set -euo pipefail
BASE="vendor/github.com/fxamacker/cbor/v2"
DECODE="$BASE/decode.go"
echo "== Find defaulting logic for opts.DefaultByteStringType =="
rg -n "DefaultByteStringType.*== nil|== nil.*DefaultByteStringType|defaultByteStringType.*nil" "$DECODE"
echo
echo "== Show any code that assigns []byte when DefaultByteStringType is nil (search for typeByteSlice usage) =="
rg -n "typeByteSlice|defaultByteStringType.*typeByteSlice|DefaultByteStringType.*typeByteSlice" "$DECODE"
echo
echo "== Show mapping for DefaultByteStringType usage in parse() byte-string branch (effectiveByteStringType nil => typeByteSlice) =="
rg -n "effectiveByteStringType.*defaultByteStringType|if effectiveByteStringType == nil" "$DECODE"Repository: openshift/openshift-mcp-server
Length of output: 793
Fix duplicate-key tracking for unmatched CBOR byte-string struct keys (avoid string(keyBytes) normalization)
parseMapToStruct forwards unmatched keys for both text-string and byte-string cases to handleUnmatchedMapKey as string(keyBytes), even when the original CBOR key was a byte string. Since handleUnmatchedMapKey deduplicates via map[any]struct{} indexing (no hashability/comparability check), this forces byte-string keys into comparable strings and can:
- conflate text keys with byte-string keys (
"foo"vsh'666f6f'), and - report duplicates among byte-string keys even though the
DupMapKeyModecontract says duplicates should depend on decoding tointerface{}and comparing with(==)—for defaultDecOptions, CBOR byte strings decode to[]byte, which is not comparable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@vendor/github.com/fxamacker/cbor/v2/decode.go` around lines 2764 - 2801,
parseMapToStruct currently calls handleUnmatchedMapKey with string(keyBytes) for
both text and byte-string keys, which normalizes byte-string keys into strings
and breaks duplicate-key tracking and type semantics; change parseMapToStruct so
it preserves the original key representation when calling handleUnmatchedMapKey
(i.e., pass the []byte keyBytes directly when keyType == cborTypeByteString and
only convert to string for cborTypeTextString), and update handleUnmatchedMapKey
(and its internal duplicate-tracking) to accept the raw key plus keyType (or an
interface{} rawKey) and deduplicate using type-aware comparison (use bytes.Equal
for []byte keys and normal map indexing or string equality for text keys)
instead of relying on map[any]struct{} with forced string normalization; refer
to parseMapToStruct, keyType, keyBytes, handleUnmatchedMapKey and DupMapKeyMode
in your changes.
| resp = []byte("'" + sanitizer.Replace(err.Error()) + "'") | ||
| } else { | ||
| resp, _ = json.Marshal(o.Response) | ||
| resp, _ = json.Marshal(o.Response) //nolint:errchkjson // error swallowed as this is our last best effort attempt |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="vendor/github.com/go-openapi/runtime/client_response.go"
# Show relevant region around line 62 (with line numbers)
nl -ba "$FILE" | sed -n '40,90p'
# Find sanitizer usage in the same file
rg -n "sanitizer" "$FILE" || true
# Find where resp is used after assignment
rg -n "resp" "$FILE" | head -n 80Repository: openshift/openshift-mcp-server
Length of output: 117
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="vendor/github.com/go-openapi/runtime/client_response.go"
# Show relevant region around the reported line (approx 62)
cat -n "$FILE" | sed -n '40,90p'
# Find sanitizer usage in the file (if any)
rg -n "sanitizer" "$FILE" || true
# Find resp usage after assignment (limit output)
rg -n "resp" "$FILE" | head -n 80Repository: openshift/openshift-mcp-server
Length of output: 3350
Handle json.Marshal errors instead of discarding them in APIError.Error().
In vendor/github.com/go-openapi/runtime/client_response.go (around line 62), resp, _ = json.Marshal(o.Response) swallows marshal failures, so the returned Error() message loses the underlying marshaling error details.
Suggested fix
- resp, _ = json.Marshal(o.Response) //nolint:errchkjson // error swallowed as this is our last best effort attempt
+ var marshalErr error
+ resp, marshalErr = json.Marshal(o.Response)
+ if marshalErr != nil {
+ resp = []byte("'" + sanitizer.Replace(marshalErr.Error()) + "'")
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@vendor/github.com/go-openapi/runtime/client_response.go` at line 62, In
APIError.Error() replace the ignored json.Marshal call so its error is checked:
call json.Marshal(o.Response) capturing (resp, err), and if err != nil include
that error text in the returned error message (or use a clear fallback message
that embeds err.Error()); ensure resp is only used when marshal succeeds and
that o.Response is still referenced in the constructed string when appropriate.
This updates the marshalling of o.Response to surface marshal failures instead
of swallowing them.
Comment on lines
+769
to
+776
| producers := route.Producers | ||
| prod, ok := producers[format] | ||
| if !ok { | ||
| if !ok { | ||
| prods := c.api.ProducersFor(normalizeOffers([]string{c.api.DefaultProduces()})) | ||
| pr, ok := prods[c.api.DefaultProduces()] | ||
| if !ok { | ||
| panic(fmt.Errorf("%d: %s", http.StatusInternalServerError, cantFindProducer(format))) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file
FILE="vendor/github.com/go-openapi/runtime/middleware/context.go"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
# Show the referenced region with line numbers
echo "=== Context around lines 740-820 ($FILE) ==="
nl -ba "$FILE" | sed -n '740,820p'
# Find other producer lookups in this file
echo
echo "=== Search for producers[...] and normalizeOffer usage ==="
rg -n "producers\[[^\]]+\]" "$FILE" || true
rg -n "normalizeOffer\(" "$FILE" || true
rg -n "normalizeOffers\(" "$FILE" || true
# Show helper definitions nearby if they exist in this file
echo
echo "=== Search for normalizeOffer/normalizeOffers definitions (in repo) ==="
rg -n "func normalizeOffer|func normalizeOffers|normalizeOffer|normalizeOffers" "$FILE"
# Also find where 'format' is set/negotiated
echo
echo "=== Search for where 'format' is assigned/negotiated ==="
rg -n "\bformat\b.*:=" "$FILE" || true
rg -n "\bformat\b" "$FILE" | head -n 80Repository: openshift/openshift-mcp-server
Length of output: 234
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="vendor/github.com/go-openapi/runtime/middleware/context.go"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE" >&2
exit 1
fi
echo "=== Context around lines 740-820 ($FILE) ==="
cat -n "$FILE" | sed -n '740,820p'
echo
echo "=== Search for producers[...] usage in the file ==="
rg -n "producers\[[^\]]+\]" "$FILE" || true
echo
echo "=== Search for normalizeOffer(s) usage in the file ==="
rg -n "normalizeOffer|normalizeOffers" "$FILE" || true
echo
echo "=== Search for where 'format' is negotiated/assigned in the file ==="
rg -n "\bformat\b" "$FILE" | head -n 120 || true
echo
echo "=== Search for normalizeOffer/normalizeOffers definitions (anywhere in repo) ==="
rg -n "func (normalizeOffer|normalizeOffers)\b" . || trueRepository: openshift/openshift-mcp-server
Length of output: 6155
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="vendor/github.com/go-openapi/runtime/middleware/typeutils.go"
if [ -f "$FILE" ]; then
echo "=== $FILE ==="
cat -n "$FILE"
else
echo "Missing $FILE" >&2
fi
echo
echo "=== Context around respondWithResponder / respondWithoutCode / respondWithCode ==="
CONTEXT_FILE="vendor/github.com/go-openapi/runtime/middleware/context.go"
# Print the relevant surrounding functions
sed -n '660,820p' "$CONTEXT_FILE" | cat -nRepository: openshift/openshift-mcp-server
Length of output: 7306
Normalize negotiated format before looking up route.Producers in respondWithCode
route.Producers is keyed by the bare media type (no ;... params), but respondWithCode looks up with raw format, so values like text/plain; charset=utf-8 won’t match and will incorrectly fall back/panic.
Suggested fix
- prod, ok := producers[format]
+ prod, ok := producers[normalizeOffer(format)]Also the same unnormalized producers[format] lookup exists in respondWithoutCode.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@vendor/github.com/go-openapi/runtime/middleware/context.go` around lines 769
- 776, The lookups against route.Producers in respondWithCode (and the similar
lookup in respondWithoutCode) use the raw format string which may contain
media-type parameters (e.g. "text/plain; charset=utf-8") and thus miss keys that
are stored as bare media types; before using producers[format] normalize the
negotiated format to the bare media type (strip any "; ..." params) — for
example call the same normalization used elsewhere (e.g. normalizeOffers or use
mime.ParseMediaType) to produce a key like "text/plain" and then use that
normalized value for the lookup and any fallback logic that calls
c.api.ProducersFor or cantFindProducer.
Comment on lines
+352
to
+357
| if len(d.routers) == 0 { | ||
| if Debug { | ||
| d.debugLogf("there are no known routers") | ||
| } | ||
| panic("internal error: no router is configured") | ||
| } |
There was a problem hiding this comment.
Avoid panicking on empty router state in request path
defaultRouter.Lookup panics when len(d.routers) == 0, but defaultRouteBuilder.Build() can legitimately produce an empty router set when the spec yields no records. That turns a normal miss into a request-time crash instead of allowing the existing NotFound/MethodNotAllowed handling to proceed.
Suggested fix
if len(d.routers) == 0 {
if Debug {
d.debugLogf("there are no known routers")
}
- panic("internal error: no router is configured")
+ return nil, false
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if len(d.routers) == 0 { | |
| if Debug { | |
| d.debugLogf("there are no known routers") | |
| } | |
| panic("internal error: no router is configured") | |
| } | |
| if len(d.routers) == 0 { | |
| if Debug { | |
| d.debugLogf("there are no known routers") | |
| } | |
| return nil, false | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@vendor/github.com/go-openapi/runtime/middleware/router.go` around lines 352 -
357, defaultRouter.Lookup currently panics when len(d.routers) == 0 which
converts a legitimate empty-router case (e.g. when defaultRouteBuilder.Build()
produces no routes) into a request-time crash; change the Lookup behavior to not
panic on an empty d.routers but instead treat it as a normal miss—keep the Debug
debugLogf call if desired, remove the panic, and return the appropriate "not
found" result (so callers receive a nil/no-match result and existing
NotFound/MethodNotAllowed handling can proceed). Ensure you modify the logic in
defaultRouter.Lookup (and any closely related return path expectations) rather
than changing defaultRouteBuilder.Build.
Comment on lines
+25
to
+29
| table: | ||
| description: |- | ||
| Table contains the routing table for this processor. | ||
| Required. | ||
| type: array |
There was a problem hiding this comment.
table is documented as required but not enforced by the JSON schema
connector_routing.yaml defines properties.table (with description “Required.”) but the root object has no top-level required: [table], so configs that omit table can still pass schema validation. Add a root-level required entry for table.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/connector_routing.yaml`
around lines 25 - 29, The schema documents properties.table as required but
doesn't enforce it; update the root object in connector_routing.yaml to include
a top-level required array that lists "table" (i.e., add required: ["table"]
alongside the existing properties definition) so that the JSON schema fails
validation when the table property is omitted.
Comment on lines
+39
to
+40
| - `profiles_endpoint` (no default): The target URL to send profile data to (e.g.: https://example.com:4318/v1development/profiles). | ||
| If this setting is present the `endpoint` setting is ignored for logs. |
There was a problem hiding this comment.
Correct profiles_endpoint path and signal description.
Line 39 should reference /v1/profiles (not /v1development/profiles), and Line 40 should say endpoint is ignored for profiles, not logs.
Suggested patch
-- `profiles_endpoint` (no default): The target URL to send profile data to (e.g.: https://example.com:4318/v1development/profiles).
- If this setting is present the `endpoint` setting is ignored for logs.
+- `profiles_endpoint` (no default): The target URL to send profile data to (e.g.: https://example.com:4318/v1/profiles).
+ If this setting is present the `endpoint` setting is ignored for profiles.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - `profiles_endpoint` (no default): The target URL to send profile data to (e.g.: https://example.com:4318/v1development/profiles). | |
| If this setting is present the `endpoint` setting is ignored for logs. | |
| - `profiles_endpoint` (no default): The target URL to send profile data to (e.g.: https://example.com:4318/v1/profiles). | |
| If this setting is present the `endpoint` setting is ignored for profiles. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlp_http.md`
around lines 39 - 40, The documentation for profiles_endpoint contains an
incorrect example path and wrong signal reference; update the description for
profiles_endpoint to use the correct path "/v1/profiles" instead of
"/v1development/profiles" and change the sentence that currently says the
`endpoint` setting is ignored "for logs" to state it is ignored "for profiles";
locate this in the exporter_otlp_http.md entry for `profiles_endpoint` and
adjust the example URL and the final clause accordingly.
Comment on lines
+167
to
+173
| sizer: | ||
| description: |- | ||
| Sizer determines the type of size measurement used by the batch. | ||
| If not configured, use the same configuration as the queue. | ||
| It accepts "requests", "items", or "bytes". | ||
| type: object | ||
| additionalProperties: false |
There was a problem hiding this comment.
sizer schema type conflicts with its documented values.
Line 172 and Line 191 set sizer as type: object, but both descriptions explicitly accept string values ("requests", "items", "bytes"). This will validate the wrong shape.
Suggested schema fix
sizer:
description: |-
Sizer determines the type of size measurement used by the batch.
If not configured, use the same configuration as the queue.
It accepts "requests", "items", or "bytes".
- type: object
+ type: string
+ enum: [requests, items, bytes]
@@
sizer:
description: |-
Sizer determines the type of size measurement used by this component.
It accepts "requests", "items", or "bytes".
- type: object
+ type: string
+ enum: [requests, items, bytes]Also applies to: 187-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlp_http.yaml`
around lines 167 - 173, The schema declares the "sizer" field as type: object
but the description says it should accept the string values "requests", "items",
or "bytes"; change the "sizer" schema (both occurrences labeled sizer) to type:
string and add an enum with the allowed values ["requests","items","bytes"] (or
otherwise constrain it to string values) so the validator enforces the
documented options instead of expecting an object.
Comment on lines
+113
to
+118
| sizer: | ||
| description: |- | ||
| Sizer determines the type of size measurement used by the batch. | ||
| If not configured, use the same configuration as the queue. | ||
| It accepts "requests", "items", or "bytes". | ||
| type: object |
There was a problem hiding this comment.
sizer fields have the wrong schema type.
Both sending_queue.batch.sizer and sending_queue.sizer are described as string enums but currently typed as object, which breaks validation for valid configs.
Suggested fix
sizer:
description: |-
Sizer determines the type of size measurement used by the batch.
If not configured, use the same configuration as the queue.
It accepts "requests", "items", or "bytes".
- type: object
+ type: string
+ enum: [requests, items, bytes]
@@
sizer:
description: |-
Sizer determines the type of size measurement used by this component.
It accepts "requests", "items", or "bytes".
- type: object
+ type: string
+ enum: [requests, items, bytes]Also applies to: 133-137
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlp.yaml`
around lines 113 - 118, The schema for the sizer entries is incorrect: change
the type for both sending_queue.batch.sizer and sending_queue.sizer from
"object" to "string" and add an enum with the allowed values "requests",
"items", and "bytes" so validation accepts those string enums; locate the sizer
blocks (symbols: sending_queue.batch.sizer and sending_queue.sizer) in the YAML
and update their type and enum accordingly.
Comment on lines
+167
to
+172
| sizer: | ||
| description: |- | ||
| Sizer determines the type of size measurement used by the batch. | ||
| If not configured, use the same configuration as the queue. | ||
| It accepts "requests", "items", or "bytes". | ||
| type: object |
There was a problem hiding this comment.
Fix sizer schema types to match accepted values.
Line 167 and Line 187 define sizer as type: object, but both descriptions state allowed values are "requests", "items", or "bytes" (string enum). This schema currently mismatches the documented config shape.
🔧 Proposed schema fix
sizer:
description: |-
Sizer determines the type of size measurement used by the batch.
If not configured, use the same configuration as the queue.
It accepts "requests", "items", or "bytes".
- type: object
+ type: string
+ enum:
+ - requests
+ - items
+ - bytes
@@
sizer:
description: |-
Sizer determines the type of size measurement used by this component.
It accepts "requests", "items", or "bytes".
- type: object
+ type: string
+ enum:
+ - requests
+ - items
+ - bytesAlso applies to: 187-191
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/exporter_otlphttp.yaml`
around lines 167 - 172, The schema declares the "sizer" field as type: object
but its description and allowed values indicate it should be a string enum;
update both occurrences of the "sizer" schema to use type: string and add an
enum: ["requests", "items", "bytes"] (and keep the existing description) so the
YAML matches the documented config shape for the sizer field.
Comment on lines
+17
to
+19
| tokens: | ||
| description: Tokens specifies multiple bearer tokens to use for every RPC. | ||
| type: string |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Inspect local vendored docs for this schema
fd -i 'extension_bearertokenauth.md' | xargs -r sed -n '1,220p'
# 2) Inspect schema line in this file for quick comparison
sed -n '1,120p' vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_bearertokenauth.yamlRepository: openshift/openshift-mcp-server
Length of output: 12646
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find likely Go config source for bearertokenauth
rg -n --hidden --no-ignore-vcs "bearertokenauth" vendor 2>/dev/null | head -n 50 || true
# Look specifically for config field type patterns
rg -n --hidden --no-ignore-vcs "tokens\s*\[\]string|Tokens\s*\[\]string|Tokens\s+string|tokens\s+string" vendor 2>/dev/null | head -n 50 || true
# Try to locate a bearertokenauth config struct
rg -n --hidden --no-ignore-vcs "type\s+.*Bearer.*Token.*Auth|type\s+.*Bearer.*Token.*" vendor 2>/dev/null | head -n 50 || true
# Re-print the exact local schema property block for context
sed -n '1,80p' vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_bearertokenauth.yaml | sed -n '1,80p'Repository: openshift/openshift-mcp-server
Length of output: 12056
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the vendored bearertokenauth extension source (Go code)
fd -i "bearertokenauthextension" vendor | head -n 50 || true
fd -i "bearertokenauth" vendor | head -n 50 || true
# Search for the config struct / field definitions inside that extension
EXT_DIR="$(fd -i "bearertokenauthextension" vendor | head -n 1 || true)"
if [[ -n "${EXT_DIR:-}" ]]; then
echo "Using EXT_DIR=$EXT_DIR"
rg -n --hidden --no-ignore-vcs "type .*Config|struct .*Config|Tokens|tokens" "$EXT_DIR" | head -n 200
rg -n --hidden --no-ignore-vcs "Tokens\s*\[\]string|Tokens\s+string|\bTokens\b.*[]" "$EXT_DIR" | head -n 200
fi
# Also search the whole vendor for the specific config struct name if present
rg -n --hidden --no-ignore-vcs "BearerTokenAuth|bearertokenauth" vendor/github.com/open-telemetry/opentelemetry-collector-contrib/extension 2>/dev/null | head -n 200 || trueRepository: openshift/openshift-mcp-server
Length of output: 4578
🌐 Web query:
site:github.com opentelemetry-collector-contrib bearertokenauth tokens type
💡 Result:
In the OpenTelemetry Collector Contrib repository, the bearertokenauth extension allows you to provide authentication tokens via a few configuration options, including the tokens field [1][2]. The tokens configuration setting expects a list of static authorization strings [1][2]. Each entry in the list is a string [3]. Regarding the configuration: - The tokens option is defined as: tokens: [string] [1][2]. - If you use the tokens field, one of the provided tokens will be used for client calls [1][2]. - You can provide either a single token (using the token field), a list of tokens (using the tokens field), or a file containing a token (using the filename field) [1][3][2]. - If both filename and token(s) are specified, the filename field takes precedence and the token/tokens fields are ignored [1][3][2]. - Regardless of the source, the token value is prepended by the configured scheme (which defaults to "Bearer") before being sent in the request header or gRPC metadata [1][2]. For example: extensions: bearertokenauth/multipletokens: scheme: "Bearer" tokens: - "token1" - "token2"
Citations:
- 1: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.147.0/extension/bearertokenauthextension/README.md
- 2: https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/extension/bearertokenauthextension
- 3: https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/extension/bearertokenauthextension/bearertokenauth.go
Fix bearertokenauth tokens config schema type (string vs array)
vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_bearertokenauth.yaml declares tokens as type: string while upstream bearertokenauthextension defines tokens as a list of strings (YAML array, e.g. tokens: ["token1", "token2"]). Update the schema to an array type (with items: { type: string }) so it matches the config contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@vendor/github.com/os-observability/redhat-opentelemetry-collector/configschemas/schemas/0.144.0/extension_bearertokenauth.yaml`
around lines 17 - 19, The schema declares the tokens field with the wrong type:
change the "tokens" property in extension_bearertokenauth.yaml from type: string
to an array of strings by setting type: array and adding items: { type: string }
so it matches the upstream bearertokenauthextension contract; update the
"tokens" mapping (the tokens property) accordingly to validate YAML lists like
tokens: ["token1","token2"].
Member
Author
|
@matzew Can you help with review? |
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
2 participants
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.
This update also brings the new toolset otelcol. Evals are also updated
Summary by CodeRabbit
Release Notes
Documentation
Tests
Dependencies
obs-mcpto v0.3.0 and vendored packages for enhanced functionality.