TT-17154 Clarify config struct comments for docs generation#69
Merged
Conversation
Improve user-facing documentation generated from struct comments based on PR review feedback: - Exporter: fix grammar; describe it as the OTLP transport that must match the collector's protocol. - ResourceName: replace circular wording with concrete meaning (service name reported as the OpenTelemetry resource that identifies the Gateway in backends). - OpenTelemetry.Enabled and TLS.Enable: adopt Tyk's standard "Set this to true to enable..." phrasing for consistency with the rest of gateway config. - TLS.CAFile: clarify that only filesystem paths are accepted; certificate store IDs are not supported here. - TLS.MaxVersion/MinVersion: clarify that the "1.3"/"1.2" defaults are enforced by the library, not inherited from Go's crypto/tls.
mativm02
commented
May 19, 2026
| // This should be set to the same type of the OpenTelemetry collector. | ||
| // Valid values are "grpc", or "http". | ||
| // Defaults to "grpc". | ||
| // Transport used to send telemetry data to the OpenTelemetry collector |
Contributor
Author
There was a problem hiding this comment.
mativm02
commented
May 19, 2026
| // Timeout for establishing a connection to the collector. | ||
| // Defaults to 1 second. | ||
| ConnectionTimeout int `json:"connection_timeout"` | ||
| // Name of the resource that will be used to identify the resource. |
Contributor
Author
There was a problem hiding this comment.
mativm02
commented
May 19, 2026
| } | ||
|
|
||
| type OpenTelemetry struct { | ||
| // A flag that can be used to enable or disable the trace exporter. |
Contributor
Author
There was a problem hiding this comment.
mativm02
commented
May 19, 2026
| } | ||
|
|
||
| type TLS struct { | ||
| // Flag that can be used to enable TLS. Defaults to false (disabled). |
Contributor
Author
There was a problem hiding this comment.
mativm02
commented
May 19, 2026
| // Flag that can be used to skip TLS verification if TLS is enabled. | ||
| // Defaults to false. | ||
| InsecureSkipVerify bool `json:"insecure_skip_verify"` | ||
| // Path to the CA file. |
Contributor
Author
There was a problem hiding this comment.
mativm02
commented
May 19, 2026
| CertFile string `json:"cert_file"` | ||
| // Path to the key file. | ||
| KeyFile string `json:"key_file"` | ||
| // Maximum TLS version that is supported. |
Contributor
Author
There was a problem hiding this comment.
Drop the Tyk-vs-Go-crypto/tls parenthetical from the version defaults comments; the simpler "Defaults to" form matches the rest of the file.
The original wording is already aligned with the tyk-docs version (tyk-docs PR #2018), so there's no docs-consistency reason to change it.
sharadregoti
approved these changes
May 20, 2026
andyo-tyk
requested changes
May 20, 2026
andyo-tyk
left a comment
andyo-tyk
left a comment
Contributor
There was a problem hiding this comment.
Proposed changes to the comments
Co-authored-by: andyo-tyk <99968932+andyo-tyk@users.noreply.github.com>
andyo-tyk
approved these changes
May 21, 2026
4 tasks
mativm02
added a commit
to TykTechnologies/tyk
that referenced
this pull request
May 26, 2026
## Summary - Bumps `github.com/TykTechnologies/opentelemetry` from `v0.0.25` to `v0.0.26-0.20260521112223-3b0d024e6090`, pulling in [opentelemetry#69](TykTechnologies/opentelemetry#69) (merge commit `3b0d024e`). - That PR is a documentation-only change: it clarifies the config struct comments that drive user-facing OpenTelemetry docs generation (`ExporterConfig.Exporter`, `ResourceName`, `OpenTelemetry.Enabled`, `TLS.Enable`, `TLS.CAFile`). No behavior, API, or default changes. - Pinned via Go pseudo-version because no new tag was cut after `v0.0.25` for this commit. ## Motivation PR review feedback on the gateway-side OpenTelemetry config docs noted unclear/inconsistent wording and grammar issues. Since the docs are generated from the upstream library's struct comments, the fixes live in `TykTechnologies/opentelemetry`. This PR brings those fixes into the gateway. ## Test plan - [x] `go build ./...` — OTel-touching packages (`./internal/otel/...`, `./internal/graphql/...`, `./gateway/...`) compile clean. The only build noise (`test/goplugins` "main undeclared", linker duplicate `-ldl` warning) is pre-existing on master and unrelated. - [x] `go vet ./internal/otel/...` — passes. - [x] `go mod tidy` — clean, no unrelated module churn. - [ ] CI green on this branch. ## Notes for reviewers - Since the upstream change is comment-only, the diff here is minimal (2 lines in `go.mod`, 4 in `go.sum`). - Once a `v0.0.26` tag is cut on the opentelemetry repo, a follow-up can replace the pseudo-version with the clean tag — but functionally identical. <!---TykTechnologies/jira-linter starts here--> ### Ticket Details <details> <summary> <a href="https://tyktech.atlassian.net/browse/TT-17154" title="TT-17154" target="_blank">TT-17154</a> </summary> | | | |---------|----| | Status | In Code Review | | Summary | Gateway API + config 5.13.0 | Generated at: 2026-05-21 11:53:06 </details> <!---TykTechnologies/jira-linter ends here-->
mativm02
added a commit
to TykTechnologies/tyk
that referenced
this pull request
May 28, 2026
…5 doc-comment fixes (#8240) (#8249) ## Summary Cherry-pick of #8240 onto `release-5.13`. Bumps `github.com/TykTechnologies/opentelemetry` from `v0.0.25` to `v0.0.26-0.20260521112223-3b0d024e6090`, pulling in doc-comment fixes (TykTechnologies/opentelemetry#69). Documentation-only upstream change — no behavior, API, or default changes. - Original PR: #8240 - Original commit: 8a9a180 - Files changed: `go.mod` (2 lines), `go.sum` (4 lines) ## Test plan - [ ] CI green on this branch
mativm02
added a commit
to TykTechnologies/tyk
that referenced
this pull request
May 28, 2026
….25 doc-comment fixes (#8240) (#8250) ## Summary Cherry-pick of #8240 onto `release-5.13.0`. Bumps `github.com/TykTechnologies/opentelemetry` from `v0.0.25` to `v0.0.26-0.20260521112223-3b0d024e6090`, pulling in doc-comment fixes (TykTechnologies/opentelemetry#69). Documentation-only upstream change — no behavior, API, or default changes. Follow-up to #8235 (the docs update that already landed on `release-5.13.0`), bringing the matching gateway-side library bump. - Original PR: #8240 - Original commit: 8a9a180 - Files changed: `go.mod` (2 lines), `go.sum` (4 lines) ## Test plan - [ ] CI green on this branch
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
3 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.
Description
Improves user-facing documentation generated from comments on the config structs. Changes are documentation-only — no behavior, signatures, or defaults are modified.
Field-by-field changes:
ExporterConfig.Exporter— Fixed broken grammar ("to sending data in OTLP protocol") and described the field as the OTLP transport that must match the collector's protocol.ExporterConfig.ResourceName— Replaced the circular description ("Name of the resource that will be used to identify the resource") with concrete meaning: the service name reported as the OpenTelemetry resource, identifying the Gateway in backends.OpenTelemetry.EnabledandTLS.Enable— Adopted Tyk's standard"Set this to true to enable..."phrasing for consistency with the rest of the gateway config.TLS.CAFile— Clarified that only filesystem paths are accepted; certificate IDs from the Tyk certificate store are not supported here (reviewer asked whether it accepts cert IDs).Motivation and Context
PR review feedback on the gateway-side OpenTelemetry config documentation noted that several generated doc entries were unclear, used inconsistent phrasing, or had grammar issues. Because the docs are generated from these struct comments, the fixes belong in this library.
Out of scope
One review comment asked why TLS settings here are not part of
ExternalServiceConfig(the centralized external-service config in the gateway). That's an architectural question — folding the library's TLS struct intoExternalServiceConfigwould require coordination with other consumers of this library and isn't a doc-only fix. Tracking that separately.The
Tracesfield comment fix (replacing "non-nil" with "when set") lives in thetykrepo and is being handled as a separate change there.Test Coverage For This Change
Comment-only change. Verified locally:
go build ./...— compiles clean.gofmt -l config/config.go— no formatting issues.Types of changes
Checklist
main.main.gofmt -s -w .,go vet ./...).