docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages - #326

Merged
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes
May 19, 2026
Merged

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages#326
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes

Conversation

@janiussyafiq

@janiussyafiqjaniussyafiq commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes six new-developer onboarding gaps surfaced by an end-to-end walkthrough of the 7-page onboarding flow (docs/index.mddocs/configuration/bootstrap-config.md) against main at 1a744ee. Three of the fixes (#1, #2, #3) close hard blockers on a clean machine (missing cargo / npm / provider-API-key prerequisites; model_not_found from display_name/model_name ambiguity; an Observability Exporter section whose second sentence is style-guide advice rather than user content). The remaining three (#4, #5, #6) close friction points along the same path (bootstrap-config.md field tables missing a Default column and an observability subsection; two credential-handling operational callouts; a cold-build duration note).

Doc-only diff. No code, schemas, configs, or test fixtures touched.

Closes#325.

Changes

FileChange
docs/quickstart/self-hosted.mdPrerequisites: add Rust 1.93+ with cargo (via rustup) as the leading prerequisite. After cargo run …: add a one-sentence note that the first-time build typically takes 3–5 minutes and that subsequent runs are incremental.
docs/quickstart/first-model-first-key-first-request.mdPrerequisites: replace "A reachable upstream OpenAI-compatible endpoint" with explicit credential-acquisition guidance (supported providers, sign-up link with cost order-of-magnitude, sk-... format hint, placeholder pointer). After the ProviderKey create-time response example: add a :::warning callout that the secret field is plaintext and that GET /admin/v1/provider_keys/:id also returns the plaintext.
docs/quickstart/openai-sdk.mdNew ## Prerequisites section placed between the "If you have not done that yet…" intro and the first ## What Changes In The SDK section. Lists a running gateway with provider key / model / API key already created, and Node.js 18+ with npm (the OpenAI SDK floor).
docs/overview/core-concepts.mdModel field-list bullet: inline-annotate display_name as the alias callers send in the API request's model field and model_name as the upstream model ID forwarded to the provider. API Key section: append a sentence after the key_hash description noting the plaintext is unrecoverable post-create and pointing at POST /admin/v1/apikeys/:id/rotate. Observability Exporter section: replace the documentation-author-facing second sentence with a user-facing description of what the resource does (OTLP-compatible export to external backends) and when to configure one.
docs/configuration/bootstrap-config.mdAdd a Default column to the field tables on etcd, proxy, admin. Build out the observability section with a new field table covering service_name, log_level, access_log, metrics.prometheus.*, metrics.otlp.*, and tracing.otlp.*, plus a pointer to the dynamic Observability Exporters page. Build out the cache section with a small field table covering backend and redis.

Net diff: 5 files, +58 / -35. Source: git diff --stat.

Test plan

Doc-only diff — no .rs files, schemas, configs, or test fixtures touched. The cargo trio was still run end-to-end as the canonical pre-merge gate.

  • cargo fmt --check — PASS (exit 0).
  • cargo clippy --workspace --all-targets -- -D warnings — PASS (exit 0; full tail captured below).
  • cargo test --workspace — PASS (exit 0; full crate-suite results captured below).
  • grep -rln <each affected page> tests/e2e/ for all 5 pages returns empty. pnpm test under tests/e2e/ is not applicable for this diff.

cargo clippy tail

 Checking aisix-proxy v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-proxy)
Checking aisix-admin v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-admin)
Checking aisix-server v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-server)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 40.28s

cargo test summary

33 test-suite-result lines, all 'ok'. Aggregate: 1029 tests passed, 0 failed, 3 ignored across the workspace (unit suites + doc-tests). Sample tail (last 3 named tests + last result line):
test keyword::tests::invalid_regex_is_a_clean_error_not_a_panic ... ok
test usage::tests::full_channel_drop_does_not_panic ... ok
test dispatch::tests::build_v1_url_rejects_path_without_leading_slash - should panic ... ok
test result: ok. <N> passed; 0 failed; <N> ignored; 0 measured; 0 filtered out

Affected pages

  • docs/quickstart/self-hosted.md
  • docs/quickstart/first-model-first-key-first-request.md
  • docs/quickstart/openai-sdk.md
  • docs/overview/core-concepts.md
  • docs/configuration/bootstrap-config.md

Pre-merge-check verification log

The three pre-merge checks called out in #325 were resolved as follows:

  1. Rust / Node minimum versions. Verified Rust against rust-toolchain.toml (channel = "1.93.1") and Cargo.toml (rust-version = "1.93"). The issue body's tentative "Rust 1.79+" floor was bumped to Rust 1.93 or newer in the doc, and a note added that rustup will pull the right channel automatically because rust-toolchain.toml pins it. For Node.js, neither tests/e2e/package.json nor a top-level package.json declares an engines floor, so the doc cites Node.js 18 or newer (the OpenAI SDK's published floor at version 4.x shipped in tests/e2e/package.json).
  2. Bootstrap-config Default column. Pulled all default values from crates/aisix-core/src/config.rs (this repo's config crate lives in aisix-core, not in a separate aisix-config crate as the issue body tentatively listed). Values pulled: EtcdConfig::default_prefix → "/aisix", default_dial_timeout / default_request_timeout → 5000ms, ProxyConfig::default_body_limit → 10 * 1024 * 1024, AdminConfig::default → addr "127.0.0.1:0" / admin_keys [] / tls None, ObservabilityConfig::default_service_name → "aisix", default_log_level → "info", default_access_log → true, PrometheusConfig::default → { enabled: true, path: "/metrics" }, OtlpConfig::default → { enabled: false, endpoint: None }, OtlpTracingConfig::default → { enabled: false, endpoint: None, sample_ratio: 1.0 }, CacheConfig::default → { backend: Memory, redis: None }. For fields without an explicit default and with Config::validate enforcement (etcd.endpoints, proxy.addr), the column reads required. For fields whose Default is None, the column reads none.
  3. Provider-key GET response shape.crates/aisix-core/src/models/provider_key.rs:39 defines pub secret: String without #[serde(skip_serializing)], and crates/aisix-admin/src/provider_keys_handlers.rs:29-38 (get_provider_key) returns Json<ResourceEntry<ProviderKey>> — so GET /admin/v1/provider_keys/:iddoes include the plaintext secret in the response body, every time. The original draft callout in docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages #325 read "subsequent reads via GET do not include it", which was empirically incorrect. The callout in this PR was adjusted to reflect actual behavior: the warning now states that the plaintext is in both the create response and any subsequent GET, and that the same handling applies any time you read the resource.

References

Summary by CodeRabbit

  • Documentation
    • Converted narrative configuration sections into structured tables for bootstrap, observability, and cache settings (including defaults and required/optional status)
    • Clarified core concepts: model display vs upstream identifiers, API key lifecycle/rotation, and observability exporter behavior
    • Added quickstart prerequisites: upstream provider API key, Node.js 18+ for OpenAI SDK, Rust 1.93+ for self-hosted
    • Added warning to treat provider API secrets as sensitive

Review Change Stack

CopilotAI review requested due to automatic review settings May 18, 2026 04:04
@janiussyafiqjaniussyafiq added documentation Improvements or additions to documentation priority-normal labels May 18, 2026
@coderabbitai

coderabbitaiBot commented May 18, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 2f8aa0cb-5faa-4a13-a703-ebe2423802be

📥 Commits

Reviewing files that changed from the base of the PR and between 5401ff1 and 1094aac.

📒 Files selected for processing (1)
  • docs/configuration/bootstrap-config.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/configuration/bootstrap-config.md

📝 Walkthrough

Walkthrough

This PR updates documentation: restructures bootstrap configuration sections into tables with defaults and optional flags, refines core concept fields and observability exporter text, and adds explicit quickstart prerequisites plus an API-key secrecy warning.

Changes

Documentation Clarity Improvements

Layer / File(s)Summary
Bootstrap configuration tables
docs/configuration/bootstrap-config.md
etcd, proxy, admin, observability, and cache "Important fields" are converted from narrative lists into tables that show default values, optional fields (e.g., tls), reserved telemetry keys, and conditional nested blocks (e.g., redis when backend: redis).
Core concept refinements
docs/overview/core-concepts.md
Model section now documents display_name, provider, and model_name; API Key section notes plaintext bearer tokens cannot be retrieved after creation and must be rotated; Observability Exporter text focuses on OTLP-compatible telemetry export.
Quickstart prerequisites and security guidance
docs/quickstart/openai-sdk.md, docs/quickstart/self-hosted.md, docs/quickstart/first-model-first-key-first-request.md
OpenAI SDK quickstart adds a Prerequisites section (gateway + Node.js 18+); self-hosted quickstart requires Rust 1.93+ and notes first cargo run compile time; first-model guide adds upstream provider API key prerequisite and warns that admin API returns secret in plaintext and it must be treated as sensitive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This doc-only PR improves new-developer onboarding by clarifying prerequisites, model naming, credential handling, and bootstrap configuration defaults across the quickstart and overview/configuration docs.

Changes:

  • Adds Rust/Node/provider-key prerequisite guidance and build-time expectations.
  • Clarifies model alias vs upstream model ID and credential visibility.
  • Expands bootstrap configuration tables, especially observability and cache fields.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
FileDescription
docs/quickstart/self-hosted.mdAdds Rust/cargo prerequisite and first-build duration note.
docs/quickstart/openai-sdk.mdAdds gateway and Node.js/npm prerequisites.
docs/quickstart/first-model-first-key-first-request.mdClarifies provider API key prerequisite and plaintext provider-key handling.
docs/overview/core-concepts.mdClarifies model fields, API-key plaintext handling, and observability exporter description.
docs/configuration/bootstrap-config.mdAdds defaults and expanded observability/cache bootstrap field tables.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threaddocs/overview/core-concepts.md Outdated

Current data-plane behavior is based on `key_hash`, not plaintext storage. The proxy hashes the incoming bearer token and resolves it against the stored `key_hash`.

This means you cannot retrieve the plaintext bearer after creation — capture the value returned at create time, and if you lose it, use `POST /admin/v1/apikeys/:id/rotate` to issue a new one.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Rewrote the appended paragraph to reflect actual behavior: the plaintext bearer is chosen (or generated) by the caller and SHA-256-hashed locally before submission, the gateway never sees or returns the plaintext at create time, and POST /admin/v1/apikeys/:id/rotate is the only endpoint that emits a server-generated plaintext (verified against apikeys_handlers.rs:97-104 for create and :155-178 for rotate). Pushed in commit 5401ff1.

Comment on lines +161 to +164
| `metrics.otlp.enabled` | push-style OTLP metrics exporter | `false` |
| `metrics.otlp.endpoint` | OTLP/gRPC collector endpoint, e.g. `"http://otel-collector:4317"` | none |
| `tracing.otlp.enabled` | push-style OTLP traces exporter | `false` |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces | none |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Confirmed against code: install_otlp_tracer (crates/aisix-obs/src/otlp.rs:26-58) validates the endpoint and emits a startup log line but defers the real exporter pipeline ("The real exporter wires up in a follow-up PR" per the inline code comment); no install_otlp_metrics function exists in the workspace. The bootstrap-config observability table now carries a Status column marking metrics.otlp.* as reserved (not yet wired) and tracing.otlp.* as partial (validation only). Pushed in commit 5401ff1.

Comment threaddocs/overview/core-concepts.md Outdated
An `Observability Exporter` is a resource that configures external telemetry export from the gateway.

Use this concept when documenting external metrics, traces, or event forwarding behavior.
An `Observability Exporter` ships telemetry (metrics, traces, request logs) from the gateway to an external backend over an OTLP-compatible endpoint (Grafana Tempo / Loki, Honeycomb, Langfuse via OTLP, and so on). Configure one when you want gateway request and response data forwarded to your existing observability stack.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against otlp_http_sink.rs:184/:249 (the exporter builds OTLP traces payloads from UsageEvent records — per-request spans, not general metrics or logs). Rewrote the section to: "ships per-request span telemetry — derived from gateway UsageEvent records — to an OTLP/HTTP-compatible backend". Pushed in commit 5401ff1.

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | scrape-style Prometheus exporter | `true` |
| `metrics.prometheus.path` | HTTP path the Prometheus exporter is served on | `"/metrics"` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-admin/src/lib.rs:68 (/metrics route is hardcoded with no conditional gating). The bootstrap-config observability table now marks metrics.prometheus.enabled and metrics.prometheus.path as reserved (not yet consulted), with a footnote that the Prometheus exporter is currently mounted unconditionally at the hardcoded admin /metrics path. Pushed in commit 5401ff1.

| --- | --- | --- |
| `service_name` | service-name attribute attached to every metric, log, and span emitted by the process | `"aisix"` |
| `log_level` | minimum log level (`error` / `warn` / `info` / `debug` / `trace`) | `"info"` |
| `access_log` | emit a structured access-log line for every proxy request | `true` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-obs/src/access_log.rs:32-50 (AccessLog::emit() always calls tracing::info! with no config gate) and the unconditional emit_access_log call sites in every proxy handler. The bootstrap-config observability table now marks access_log as reserved (not yet consulted), with a footnote that access logs are emitted unconditionally by every proxy handler today. Pushed in commit 5401ff1.

…lity-config claims to match runtime (Copilot review)
…s fields
PR #331 (commit e6125b6) wired the `metrics.prometheus.enabled` and
`metrics.prometheus.path` consumers in `crates/aisix-admin/src/lib.rs:154-159`
plus the `normalized_prometheus_path` helper at `:164-174`, and exercised
both paths end-to-end in three new unit tests
(`metrics_endpoint_uses_configured_path`,
`metrics_endpoint_normalizes_configured_path`,
`metrics_endpoint_can_be_disabled`).
The two rows in this PR's observability field table that called those
fields "reserved (not yet consulted)" are now factually stale. Update
both rows to `wired` and align the row descriptions with #331's
runtime behaviour. Also adjust the introductory paragraph above the
table so it no longer asserts that only `service_name` and `log_level`
are consulted at runtime.
This is the consumer-trace discipline locked in during round 2 of this
PR, applied prospectively to a moving target: PR #331 changed the
consumer site after round 2's verification window, so the doc text
needs to update.
CopilotAI review requested due to automatic review settings May 18, 2026 15:24

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

docs/configuration/bootstrap-config.md:164

  • The tracing endpoint is described as validated at boot, but the implementation only checks that tracing.otlp.enabled=true has a non-empty endpoint; it does not parse or validate the endpoint URL/protocol before returning a handle. Calling this "validated" overstates the current behavior.
| `tracing.otlp.enabled` | enabling this validates the endpoint at boot and emits a startup log line; the OTLP traces pipeline itself is deferred to a future release | `false` | partial (validation only) |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces; validated at boot when `tracing.otlp.enabled` is `true` | none | partial (validation only) |

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | controls whether the admin listener mounts the Prometheus scrape endpoint; when `false`, no `/metrics` route is registered | `true` | wired |
| `metrics.prometheus.path` | mount path for the Prometheus scrape endpoint when `metrics.prometheus.enabled` is `true`; values without a leading slash are normalised by prepending one, and an empty value falls back to `/metrics` | `"/metrics"` | wired |
## `observability`

Use `observability` to configure:
Use `observability` to set process-wide telemetry knobs: service name, log level, Prometheus exporter control, and (in future releases) access-log gating and OTLP exporters. Today `service_name`, `log_level`, and the `metrics.prometheus.*` block are consulted at runtime; the remaining keys are recognized in the schema and reserved for upcoming releases — setting them is harmless but currently has no effect.
@moonming
moonming merged commit e12155b into mainMay 19, 2026
11 checks passed
janiussyafiq added a commit that referenced this pull request May 20, 2026
Two related items folded into a single revision commit per operator
authorization (msg `a5c9dec7`):
1. Merge conflict resolution against `main` post-PR-#326
PR #326 (merge commit `e12155b`, landed 2026-05-19 02:30:04Z) updated
the `## Prerequisites` block and the `## Step 3: Start the gateway`
section of `docs/quickstart/self-hosted.md` on `main`. This branch's
prior edits (etcd glossary link, `config.yaml` location anchor,
`YOUR_ADMIN_KEY` placeholder format note, "Keep the gateway running"
framing) overlapped the Prerequisites bullet on the same physical
line.
Resolution per Umar's approved Option 2: single new commit on top of
`a109d2d` that re-applies this branch's Bucket-B edits on top of the
post-#326 canonical lines:
- Add the post-#326 Rust 1.93 prerequisite bullet.
- Capitalize "A reachable etcd instance" (per #326) while preserving
this branch's `[etcd](../overview/glossary.md#etcd)` glossary link.
- Add the post-#326 "first time you run this" cargo-build duration
paragraph between the `cargo run` code block and this branch's
"Keep the gateway running. Open a new terminal..." framing. The
two paragraphs now sequence as: (a) explain first-run build time,
(b) instruct reader to keep the gateway running and switch
terminals.
Other 4 overlapping files (`bootstrap-config.md`, `core-concepts.md`,
`first-model-first-key-first-request.md`, `openai-sdk.md`) auto-merge
cleanly against post-#326 main and are not touched by this commit.
2. Copilot review address — comment id `3260189475`
The reviewer flagged that the "Capture the id (jq path)" snippet on
`docs/quickstart/first-model-first-key-first-request.md` re-ran the
same `POST /admin/v1/provider_keys` as Step 1, which would either
trigger a duplicate-`display_name` rejection from the admin API or
leave the reader with two ProviderKey rows whose ids differ from the
response example shown after the first POST. Verified empirically by
reading the two POST blocks (lines 37-46 and the captured-id snippet)
against each other.
Restructure the section so the jq path is presented as a variant of
the original create, not a second POST:
- Rename the code block title to "Create and capture the id in one
shot" to make the variant relationship explicit.
- Wrap the two paths as bulleted Option A / Option B blocks. Option
A is the jq variant of the create above ("instead of the curl
shown earlier"). Option B is the jq-free path ("if you already
ran the curl above, copy the id field by eye").
- Prefix the section with an explicit "pick one, don't run both"
callout naming the duplicate-display_name rejection as the failure
mode if the reader runs both forms.
janiussyafiq added a commit that referenced this pull request May 20, 2026
Integrate origin/main (commit 2c1d485 = post-PR-#326 / #348 plus
#330 / #341 / #343 / #345 / #346) into this branch via `git merge
--squash` to clear PR #344's lingering `mergeable: dirty` state.
Conflict on `docs/quickstart/self-hosted.md` was a 3-way-merge-base
artifact: base (3596c0a) read `- a reachable etcd instance`, main
changed `a` → `A` (via #326), this branch additionally inserted the
glossary link. Both changes are wanted; resolution per Umar's
approved plan was `git checkout --ours`, which preserves the branch's
self-hosted.md state (already integrates capital A + glossary link
+ first-time-build paragraph + keep-running framing). Other 4
overlapping doc files auto-merged cleanly (`bootstrap-config.md`,
`core-concepts.md`, `first-model-first-key-first-request.md`,
`openai-sdk.md`). Code files all auto-merged cleanly.
Additional Copilot review (post-`167196a` cycle) addressed:
- `docs/index.md:7` — change link display text from `[data-plane]`
to `[data plane]` to match the canonical glossary term. The URL
anchor `#data-plane` stays kebab-case (matches the glossary
heading's auto-anchor); only the display text changes. Comment
id 3271145422.
- `docs/quickstart/openai-sdk.md:43` — change `All three steps below`
to `All commands below`. The Install-the-SDK section has two
command blocks (mkdir+cd, npm install), not three; the prior
wording originated from a mental model (mkdir, cd, install)
that doesn't match the typographic count of code blocks under
the heading. Comment id 3271145458.
Copilot's third comment on `docs/overview/core-concepts.md`
Observability Exporter wording (id 3271145444) auto-resolves via
this merge — main's #326 rewrite supersedes the branch's pre-#326
wording at that location ("ships per-request span telemetry…
OTLP/HTTP-compatible backend…" replaces "Use this concept when
documenting…"). No separate edit needed; the merge IS the fix.
janiussyafiq added a commit that referenced this pull request May 20, 2026
…ickstart-polish
Resolve PR #344's lingering mergeable: dirty state by linking the
branch history to origin/main (2c1d485 = post-#326 / #348 / #330 /
#341 / #343 / #345 / #346).
The squash-merge commit landed earlier (e2af197) integrated main's
content into the branch tree but did not link the histories, so
GitHub's mergeable computation still saw the 3-way-merge-base
artifact conflict on docs/quickstart/self-hosted.md (a vs A + the
glossary link / "In another terminal" vs "Keep the gateway running"
framing). This explicit merge commit ties the branch to main's
history.
Self-hosted.md conflict resolved by taking OUR side — the branch's
edits already contain main's substantive changes (capital A,
first-time-build paragraph) plus this PR's additions (glossary
link, keep-running framing, YOUR_ADMIN_KEY note, config.yaml
location anchor).
The auto-merge of first-model-first-key-first-request.md duplicated
the :::warning callout that was already integrated via the squash
commit; removed the duplicate.
moonming pushed a commit that referenced this pull request May 22, 2026
@jarvis9443
jarvis9443 deleted the docs/issue-325-onboarding-flow-fixes branch June 25, 2026 06:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationpriority-normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages

3 participants

@janiussyafiq@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages - #326

Merged
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes
May 19, 2026
Merged

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages#326
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes

Conversation

@janiussyafiq

@janiussyafiqjaniussyafiq commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes six new-developer onboarding gaps surfaced by an end-to-end walkthrough of the 7-page onboarding flow (docs/index.mddocs/configuration/bootstrap-config.md) against main at 1a744ee. Three of the fixes (#1, #2, #3) close hard blockers on a clean machine (missing cargo / npm / provider-API-key prerequisites; model_not_found from display_name/model_name ambiguity; an Observability Exporter section whose second sentence is style-guide advice rather than user content). The remaining three (#4, #5, #6) close friction points along the same path (bootstrap-config.md field tables missing a Default column and an observability subsection; two credential-handling operational callouts; a cold-build duration note).

Doc-only diff. No code, schemas, configs, or test fixtures touched.

Closes#325.

Changes

FileChange
docs/quickstart/self-hosted.mdPrerequisites: add Rust 1.93+ with cargo (via rustup) as the leading prerequisite. After cargo run …: add a one-sentence note that the first-time build typically takes 3–5 minutes and that subsequent runs are incremental.
docs/quickstart/first-model-first-key-first-request.mdPrerequisites: replace "A reachable upstream OpenAI-compatible endpoint" with explicit credential-acquisition guidance (supported providers, sign-up link with cost order-of-magnitude, sk-... format hint, placeholder pointer). After the ProviderKey create-time response example: add a :::warning callout that the secret field is plaintext and that GET /admin/v1/provider_keys/:id also returns the plaintext.
docs/quickstart/openai-sdk.mdNew ## Prerequisites section placed between the "If you have not done that yet…" intro and the first ## What Changes In The SDK section. Lists a running gateway with provider key / model / API key already created, and Node.js 18+ with npm (the OpenAI SDK floor).
docs/overview/core-concepts.mdModel field-list bullet: inline-annotate display_name as the alias callers send in the API request's model field and model_name as the upstream model ID forwarded to the provider. API Key section: append a sentence after the key_hash description noting the plaintext is unrecoverable post-create and pointing at POST /admin/v1/apikeys/:id/rotate. Observability Exporter section: replace the documentation-author-facing second sentence with a user-facing description of what the resource does (OTLP-compatible export to external backends) and when to configure one.
docs/configuration/bootstrap-config.mdAdd a Default column to the field tables on etcd, proxy, admin. Build out the observability section with a new field table covering service_name, log_level, access_log, metrics.prometheus.*, metrics.otlp.*, and tracing.otlp.*, plus a pointer to the dynamic Observability Exporters page. Build out the cache section with a small field table covering backend and redis.

Net diff: 5 files, +58 / -35. Source: git diff --stat.

Test plan

Doc-only diff — no .rs files, schemas, configs, or test fixtures touched. The cargo trio was still run end-to-end as the canonical pre-merge gate.

  • cargo fmt --check — PASS (exit 0).
  • cargo clippy --workspace --all-targets -- -D warnings — PASS (exit 0; full tail captured below).
  • cargo test --workspace — PASS (exit 0; full crate-suite results captured below).
  • grep -rln <each affected page> tests/e2e/ for all 5 pages returns empty. pnpm test under tests/e2e/ is not applicable for this diff.

cargo clippy tail

 Checking aisix-proxy v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-proxy)
Checking aisix-admin v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-admin)
Checking aisix-server v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-server)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 40.28s

cargo test summary

33 test-suite-result lines, all 'ok'. Aggregate: 1029 tests passed, 0 failed, 3 ignored across the workspace (unit suites + doc-tests). Sample tail (last 3 named tests + last result line):
test keyword::tests::invalid_regex_is_a_clean_error_not_a_panic ... ok
test usage::tests::full_channel_drop_does_not_panic ... ok
test dispatch::tests::build_v1_url_rejects_path_without_leading_slash - should panic ... ok
test result: ok. <N> passed; 0 failed; <N> ignored; 0 measured; 0 filtered out

Affected pages

  • docs/quickstart/self-hosted.md
  • docs/quickstart/first-model-first-key-first-request.md
  • docs/quickstart/openai-sdk.md
  • docs/overview/core-concepts.md
  • docs/configuration/bootstrap-config.md

Pre-merge-check verification log

The three pre-merge checks called out in #325 were resolved as follows:

  1. Rust / Node minimum versions. Verified Rust against rust-toolchain.toml (channel = "1.93.1") and Cargo.toml (rust-version = "1.93"). The issue body's tentative "Rust 1.79+" floor was bumped to Rust 1.93 or newer in the doc, and a note added that rustup will pull the right channel automatically because rust-toolchain.toml pins it. For Node.js, neither tests/e2e/package.json nor a top-level package.json declares an engines floor, so the doc cites Node.js 18 or newer (the OpenAI SDK's published floor at version 4.x shipped in tests/e2e/package.json).
  2. Bootstrap-config Default column. Pulled all default values from crates/aisix-core/src/config.rs (this repo's config crate lives in aisix-core, not in a separate aisix-config crate as the issue body tentatively listed). Values pulled: EtcdConfig::default_prefix → "/aisix", default_dial_timeout / default_request_timeout → 5000ms, ProxyConfig::default_body_limit → 10 * 1024 * 1024, AdminConfig::default → addr "127.0.0.1:0" / admin_keys [] / tls None, ObservabilityConfig::default_service_name → "aisix", default_log_level → "info", default_access_log → true, PrometheusConfig::default → { enabled: true, path: "/metrics" }, OtlpConfig::default → { enabled: false, endpoint: None }, OtlpTracingConfig::default → { enabled: false, endpoint: None, sample_ratio: 1.0 }, CacheConfig::default → { backend: Memory, redis: None }. For fields without an explicit default and with Config::validate enforcement (etcd.endpoints, proxy.addr), the column reads required. For fields whose Default is None, the column reads none.
  3. Provider-key GET response shape.crates/aisix-core/src/models/provider_key.rs:39 defines pub secret: String without #[serde(skip_serializing)], and crates/aisix-admin/src/provider_keys_handlers.rs:29-38 (get_provider_key) returns Json<ResourceEntry<ProviderKey>> — so GET /admin/v1/provider_keys/:iddoes include the plaintext secret in the response body, every time. The original draft callout in docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages #325 read "subsequent reads via GET do not include it", which was empirically incorrect. The callout in this PR was adjusted to reflect actual behavior: the warning now states that the plaintext is in both the create response and any subsequent GET, and that the same handling applies any time you read the resource.

References

Summary by CodeRabbit

  • Documentation
    • Converted narrative configuration sections into structured tables for bootstrap, observability, and cache settings (including defaults and required/optional status)
    • Clarified core concepts: model display vs upstream identifiers, API key lifecycle/rotation, and observability exporter behavior
    • Added quickstart prerequisites: upstream provider API key, Node.js 18+ for OpenAI SDK, Rust 1.93+ for self-hosted
    • Added warning to treat provider API secrets as sensitive

Review Change Stack

CopilotAI review requested due to automatic review settings May 18, 2026 04:04
@janiussyafiqjaniussyafiq added documentation Improvements or additions to documentation priority-normal labels May 18, 2026
@coderabbitai

coderabbitaiBot commented May 18, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 2f8aa0cb-5faa-4a13-a703-ebe2423802be

📥 Commits

Reviewing files that changed from the base of the PR and between 5401ff1 and 1094aac.

📒 Files selected for processing (1)
  • docs/configuration/bootstrap-config.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/configuration/bootstrap-config.md

📝 Walkthrough

Walkthrough

This PR updates documentation: restructures bootstrap configuration sections into tables with defaults and optional flags, refines core concept fields and observability exporter text, and adds explicit quickstart prerequisites plus an API-key secrecy warning.

Changes

Documentation Clarity Improvements

Layer / File(s)Summary
Bootstrap configuration tables
docs/configuration/bootstrap-config.md
etcd, proxy, admin, observability, and cache "Important fields" are converted from narrative lists into tables that show default values, optional fields (e.g., tls), reserved telemetry keys, and conditional nested blocks (e.g., redis when backend: redis).
Core concept refinements
docs/overview/core-concepts.md
Model section now documents display_name, provider, and model_name; API Key section notes plaintext bearer tokens cannot be retrieved after creation and must be rotated; Observability Exporter text focuses on OTLP-compatible telemetry export.
Quickstart prerequisites and security guidance
docs/quickstart/openai-sdk.md, docs/quickstart/self-hosted.md, docs/quickstart/first-model-first-key-first-request.md
OpenAI SDK quickstart adds a Prerequisites section (gateway + Node.js 18+); self-hosted quickstart requires Rust 1.93+ and notes first cargo run compile time; first-model guide adds upstream provider API key prerequisite and warns that admin API returns secret in plaintext and it must be treated as sensitive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This doc-only PR improves new-developer onboarding by clarifying prerequisites, model naming, credential handling, and bootstrap configuration defaults across the quickstart and overview/configuration docs.

Changes:

  • Adds Rust/Node/provider-key prerequisite guidance and build-time expectations.
  • Clarifies model alias vs upstream model ID and credential visibility.
  • Expands bootstrap configuration tables, especially observability and cache fields.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
FileDescription
docs/quickstart/self-hosted.mdAdds Rust/cargo prerequisite and first-build duration note.
docs/quickstart/openai-sdk.mdAdds gateway and Node.js/npm prerequisites.
docs/quickstart/first-model-first-key-first-request.mdClarifies provider API key prerequisite and plaintext provider-key handling.
docs/overview/core-concepts.mdClarifies model fields, API-key plaintext handling, and observability exporter description.
docs/configuration/bootstrap-config.mdAdds defaults and expanded observability/cache bootstrap field tables.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threaddocs/overview/core-concepts.md Outdated

Current data-plane behavior is based on `key_hash`, not plaintext storage. The proxy hashes the incoming bearer token and resolves it against the stored `key_hash`.

This means you cannot retrieve the plaintext bearer after creation — capture the value returned at create time, and if you lose it, use `POST /admin/v1/apikeys/:id/rotate` to issue a new one.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Rewrote the appended paragraph to reflect actual behavior: the plaintext bearer is chosen (or generated) by the caller and SHA-256-hashed locally before submission, the gateway never sees or returns the plaintext at create time, and POST /admin/v1/apikeys/:id/rotate is the only endpoint that emits a server-generated plaintext (verified against apikeys_handlers.rs:97-104 for create and :155-178 for rotate). Pushed in commit 5401ff1.

Comment on lines +161 to +164
| `metrics.otlp.enabled` | push-style OTLP metrics exporter | `false` |
| `metrics.otlp.endpoint` | OTLP/gRPC collector endpoint, e.g. `"http://otel-collector:4317"` | none |
| `tracing.otlp.enabled` | push-style OTLP traces exporter | `false` |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces | none |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Confirmed against code: install_otlp_tracer (crates/aisix-obs/src/otlp.rs:26-58) validates the endpoint and emits a startup log line but defers the real exporter pipeline ("The real exporter wires up in a follow-up PR" per the inline code comment); no install_otlp_metrics function exists in the workspace. The bootstrap-config observability table now carries a Status column marking metrics.otlp.* as reserved (not yet wired) and tracing.otlp.* as partial (validation only). Pushed in commit 5401ff1.

Comment threaddocs/overview/core-concepts.md Outdated
An `Observability Exporter` is a resource that configures external telemetry export from the gateway.

Use this concept when documenting external metrics, traces, or event forwarding behavior.
An `Observability Exporter` ships telemetry (metrics, traces, request logs) from the gateway to an external backend over an OTLP-compatible endpoint (Grafana Tempo / Loki, Honeycomb, Langfuse via OTLP, and so on). Configure one when you want gateway request and response data forwarded to your existing observability stack.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against otlp_http_sink.rs:184/:249 (the exporter builds OTLP traces payloads from UsageEvent records — per-request spans, not general metrics or logs). Rewrote the section to: "ships per-request span telemetry — derived from gateway UsageEvent records — to an OTLP/HTTP-compatible backend". Pushed in commit 5401ff1.

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | scrape-style Prometheus exporter | `true` |
| `metrics.prometheus.path` | HTTP path the Prometheus exporter is served on | `"/metrics"` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-admin/src/lib.rs:68 (/metrics route is hardcoded with no conditional gating). The bootstrap-config observability table now marks metrics.prometheus.enabled and metrics.prometheus.path as reserved (not yet consulted), with a footnote that the Prometheus exporter is currently mounted unconditionally at the hardcoded admin /metrics path. Pushed in commit 5401ff1.

| --- | --- | --- |
| `service_name` | service-name attribute attached to every metric, log, and span emitted by the process | `"aisix"` |
| `log_level` | minimum log level (`error` / `warn` / `info` / `debug` / `trace`) | `"info"` |
| `access_log` | emit a structured access-log line for every proxy request | `true` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-obs/src/access_log.rs:32-50 (AccessLog::emit() always calls tracing::info! with no config gate) and the unconditional emit_access_log call sites in every proxy handler. The bootstrap-config observability table now marks access_log as reserved (not yet consulted), with a footnote that access logs are emitted unconditionally by every proxy handler today. Pushed in commit 5401ff1.

…lity-config claims to match runtime (Copilot review)
…s fields
PR #331 (commit e6125b6) wired the `metrics.prometheus.enabled` and
`metrics.prometheus.path` consumers in `crates/aisix-admin/src/lib.rs:154-159`
plus the `normalized_prometheus_path` helper at `:164-174`, and exercised
both paths end-to-end in three new unit tests
(`metrics_endpoint_uses_configured_path`,
`metrics_endpoint_normalizes_configured_path`,
`metrics_endpoint_can_be_disabled`).
The two rows in this PR's observability field table that called those
fields "reserved (not yet consulted)" are now factually stale. Update
both rows to `wired` and align the row descriptions with #331's
runtime behaviour. Also adjust the introductory paragraph above the
table so it no longer asserts that only `service_name` and `log_level`
are consulted at runtime.
This is the consumer-trace discipline locked in during round 2 of this
PR, applied prospectively to a moving target: PR #331 changed the
consumer site after round 2's verification window, so the doc text
needs to update.
CopilotAI review requested due to automatic review settings May 18, 2026 15:24

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

docs/configuration/bootstrap-config.md:164

  • The tracing endpoint is described as validated at boot, but the implementation only checks that tracing.otlp.enabled=true has a non-empty endpoint; it does not parse or validate the endpoint URL/protocol before returning a handle. Calling this "validated" overstates the current behavior.
| `tracing.otlp.enabled` | enabling this validates the endpoint at boot and emits a startup log line; the OTLP traces pipeline itself is deferred to a future release | `false` | partial (validation only) |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces; validated at boot when `tracing.otlp.enabled` is `true` | none | partial (validation only) |

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | controls whether the admin listener mounts the Prometheus scrape endpoint; when `false`, no `/metrics` route is registered | `true` | wired |
| `metrics.prometheus.path` | mount path for the Prometheus scrape endpoint when `metrics.prometheus.enabled` is `true`; values without a leading slash are normalised by prepending one, and an empty value falls back to `/metrics` | `"/metrics"` | wired |
## `observability`

Use `observability` to configure:
Use `observability` to set process-wide telemetry knobs: service name, log level, Prometheus exporter control, and (in future releases) access-log gating and OTLP exporters. Today `service_name`, `log_level`, and the `metrics.prometheus.*` block are consulted at runtime; the remaining keys are recognized in the schema and reserved for upcoming releases — setting them is harmless but currently has no effect.
@moonming
moonming merged commit e12155b into mainMay 19, 2026
11 checks passed
janiussyafiq added a commit that referenced this pull request May 20, 2026
Two related items folded into a single revision commit per operator
authorization (msg `a5c9dec7`):
1. Merge conflict resolution against `main` post-PR-#326
PR #326 (merge commit `e12155b`, landed 2026-05-19 02:30:04Z) updated
the `## Prerequisites` block and the `## Step 3: Start the gateway`
section of `docs/quickstart/self-hosted.md` on `main`. This branch's
prior edits (etcd glossary link, `config.yaml` location anchor,
`YOUR_ADMIN_KEY` placeholder format note, "Keep the gateway running"
framing) overlapped the Prerequisites bullet on the same physical
line.
Resolution per Umar's approved Option 2: single new commit on top of
`a109d2d` that re-applies this branch's Bucket-B edits on top of the
post-#326 canonical lines:
- Add the post-#326 Rust 1.93 prerequisite bullet.
- Capitalize "A reachable etcd instance" (per #326) while preserving
this branch's `[etcd](../overview/glossary.md#etcd)` glossary link.
- Add the post-#326 "first time you run this" cargo-build duration
paragraph between the `cargo run` code block and this branch's
"Keep the gateway running. Open a new terminal..." framing. The
two paragraphs now sequence as: (a) explain first-run build time,
(b) instruct reader to keep the gateway running and switch
terminals.
Other 4 overlapping files (`bootstrap-config.md`, `core-concepts.md`,
`first-model-first-key-first-request.md`, `openai-sdk.md`) auto-merge
cleanly against post-#326 main and are not touched by this commit.
2. Copilot review address — comment id `3260189475`
The reviewer flagged that the "Capture the id (jq path)" snippet on
`docs/quickstart/first-model-first-key-first-request.md` re-ran the
same `POST /admin/v1/provider_keys` as Step 1, which would either
trigger a duplicate-`display_name` rejection from the admin API or
leave the reader with two ProviderKey rows whose ids differ from the
response example shown after the first POST. Verified empirically by
reading the two POST blocks (lines 37-46 and the captured-id snippet)
against each other.
Restructure the section so the jq path is presented as a variant of
the original create, not a second POST:
- Rename the code block title to "Create and capture the id in one
shot" to make the variant relationship explicit.
- Wrap the two paths as bulleted Option A / Option B blocks. Option
A is the jq variant of the create above ("instead of the curl
shown earlier"). Option B is the jq-free path ("if you already
ran the curl above, copy the id field by eye").
- Prefix the section with an explicit "pick one, don't run both"
callout naming the duplicate-display_name rejection as the failure
mode if the reader runs both forms.
janiussyafiq added a commit that referenced this pull request May 20, 2026
Integrate origin/main (commit 2c1d485 = post-PR-#326 / #348 plus
#330 / #341 / #343 / #345 / #346) into this branch via `git merge
--squash` to clear PR #344's lingering `mergeable: dirty` state.
Conflict on `docs/quickstart/self-hosted.md` was a 3-way-merge-base
artifact: base (3596c0a) read `- a reachable etcd instance`, main
changed `a` → `A` (via #326), this branch additionally inserted the
glossary link. Both changes are wanted; resolution per Umar's
approved plan was `git checkout --ours`, which preserves the branch's
self-hosted.md state (already integrates capital A + glossary link
+ first-time-build paragraph + keep-running framing). Other 4
overlapping doc files auto-merged cleanly (`bootstrap-config.md`,
`core-concepts.md`, `first-model-first-key-first-request.md`,
`openai-sdk.md`). Code files all auto-merged cleanly.
Additional Copilot review (post-`167196a` cycle) addressed:
- `docs/index.md:7` — change link display text from `[data-plane]`
to `[data plane]` to match the canonical glossary term. The URL
anchor `#data-plane` stays kebab-case (matches the glossary
heading's auto-anchor); only the display text changes. Comment
id 3271145422.
- `docs/quickstart/openai-sdk.md:43` — change `All three steps below`
to `All commands below`. The Install-the-SDK section has two
command blocks (mkdir+cd, npm install), not three; the prior
wording originated from a mental model (mkdir, cd, install)
that doesn't match the typographic count of code blocks under
the heading. Comment id 3271145458.
Copilot's third comment on `docs/overview/core-concepts.md`
Observability Exporter wording (id 3271145444) auto-resolves via
this merge — main's #326 rewrite supersedes the branch's pre-#326
wording at that location ("ships per-request span telemetry…
OTLP/HTTP-compatible backend…" replaces "Use this concept when
documenting…"). No separate edit needed; the merge IS the fix.
janiussyafiq added a commit that referenced this pull request May 20, 2026
…ickstart-polish
Resolve PR #344's lingering mergeable: dirty state by linking the
branch history to origin/main (2c1d485 = post-#326 / #348 / #330 /
#341 / #343 / #345 / #346).
The squash-merge commit landed earlier (e2af197) integrated main's
content into the branch tree but did not link the histories, so
GitHub's mergeable computation still saw the 3-way-merge-base
artifact conflict on docs/quickstart/self-hosted.md (a vs A + the
glossary link / "In another terminal" vs "Keep the gateway running"
framing). This explicit merge commit ties the branch to main's
history.
Self-hosted.md conflict resolved by taking OUR side — the branch's
edits already contain main's substantive changes (capital A,
first-time-build paragraph) plus this PR's additions (glossary
link, keep-running framing, YOUR_ADMIN_KEY note, config.yaml
location anchor).
The auto-merge of first-model-first-key-first-request.md duplicated
the :::warning callout that was already integrated via the squash
commit; removed the duplicate.
moonming pushed a commit that referenced this pull request May 22, 2026
@jarvis9443
jarvis9443 deleted the docs/issue-325-onboarding-flow-fixes branch June 25, 2026 06:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationpriority-normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages

3 participants

@janiussyafiq@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages - #326

Merged
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes
May 19, 2026
Merged

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages#326
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes

Conversation

@janiussyafiq

@janiussyafiqjaniussyafiq commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes six new-developer onboarding gaps surfaced by an end-to-end walkthrough of the 7-page onboarding flow (docs/index.mddocs/configuration/bootstrap-config.md) against main at 1a744ee. Three of the fixes (#1, #2, #3) close hard blockers on a clean machine (missing cargo / npm / provider-API-key prerequisites; model_not_found from display_name/model_name ambiguity; an Observability Exporter section whose second sentence is style-guide advice rather than user content). The remaining three (#4, #5, #6) close friction points along the same path (bootstrap-config.md field tables missing a Default column and an observability subsection; two credential-handling operational callouts; a cold-build duration note).

Doc-only diff. No code, schemas, configs, or test fixtures touched.

Closes#325.

Changes

FileChange
docs/quickstart/self-hosted.mdPrerequisites: add Rust 1.93+ with cargo (via rustup) as the leading prerequisite. After cargo run …: add a one-sentence note that the first-time build typically takes 3–5 minutes and that subsequent runs are incremental.
docs/quickstart/first-model-first-key-first-request.mdPrerequisites: replace "A reachable upstream OpenAI-compatible endpoint" with explicit credential-acquisition guidance (supported providers, sign-up link with cost order-of-magnitude, sk-... format hint, placeholder pointer). After the ProviderKey create-time response example: add a :::warning callout that the secret field is plaintext and that GET /admin/v1/provider_keys/:id also returns the plaintext.
docs/quickstart/openai-sdk.mdNew ## Prerequisites section placed between the "If you have not done that yet…" intro and the first ## What Changes In The SDK section. Lists a running gateway with provider key / model / API key already created, and Node.js 18+ with npm (the OpenAI SDK floor).
docs/overview/core-concepts.mdModel field-list bullet: inline-annotate display_name as the alias callers send in the API request's model field and model_name as the upstream model ID forwarded to the provider. API Key section: append a sentence after the key_hash description noting the plaintext is unrecoverable post-create and pointing at POST /admin/v1/apikeys/:id/rotate. Observability Exporter section: replace the documentation-author-facing second sentence with a user-facing description of what the resource does (OTLP-compatible export to external backends) and when to configure one.
docs/configuration/bootstrap-config.mdAdd a Default column to the field tables on etcd, proxy, admin. Build out the observability section with a new field table covering service_name, log_level, access_log, metrics.prometheus.*, metrics.otlp.*, and tracing.otlp.*, plus a pointer to the dynamic Observability Exporters page. Build out the cache section with a small field table covering backend and redis.

Net diff: 5 files, +58 / -35. Source: git diff --stat.

Test plan

Doc-only diff — no .rs files, schemas, configs, or test fixtures touched. The cargo trio was still run end-to-end as the canonical pre-merge gate.

  • cargo fmt --check — PASS (exit 0).
  • cargo clippy --workspace --all-targets -- -D warnings — PASS (exit 0; full tail captured below).
  • cargo test --workspace — PASS (exit 0; full crate-suite results captured below).
  • grep -rln <each affected page> tests/e2e/ for all 5 pages returns empty. pnpm test under tests/e2e/ is not applicable for this diff.

cargo clippy tail

 Checking aisix-proxy v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-proxy)
Checking aisix-admin v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-admin)
Checking aisix-server v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-server)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 40.28s

cargo test summary

33 test-suite-result lines, all 'ok'. Aggregate: 1029 tests passed, 0 failed, 3 ignored across the workspace (unit suites + doc-tests). Sample tail (last 3 named tests + last result line):
test keyword::tests::invalid_regex_is_a_clean_error_not_a_panic ... ok
test usage::tests::full_channel_drop_does_not_panic ... ok
test dispatch::tests::build_v1_url_rejects_path_without_leading_slash - should panic ... ok
test result: ok. <N> passed; 0 failed; <N> ignored; 0 measured; 0 filtered out

Affected pages

  • docs/quickstart/self-hosted.md
  • docs/quickstart/first-model-first-key-first-request.md
  • docs/quickstart/openai-sdk.md
  • docs/overview/core-concepts.md
  • docs/configuration/bootstrap-config.md

Pre-merge-check verification log

The three pre-merge checks called out in #325 were resolved as follows:

  1. Rust / Node minimum versions. Verified Rust against rust-toolchain.toml (channel = "1.93.1") and Cargo.toml (rust-version = "1.93"). The issue body's tentative "Rust 1.79+" floor was bumped to Rust 1.93 or newer in the doc, and a note added that rustup will pull the right channel automatically because rust-toolchain.toml pins it. For Node.js, neither tests/e2e/package.json nor a top-level package.json declares an engines floor, so the doc cites Node.js 18 or newer (the OpenAI SDK's published floor at version 4.x shipped in tests/e2e/package.json).
  2. Bootstrap-config Default column. Pulled all default values from crates/aisix-core/src/config.rs (this repo's config crate lives in aisix-core, not in a separate aisix-config crate as the issue body tentatively listed). Values pulled: EtcdConfig::default_prefix → "/aisix", default_dial_timeout / default_request_timeout → 5000ms, ProxyConfig::default_body_limit → 10 * 1024 * 1024, AdminConfig::default → addr "127.0.0.1:0" / admin_keys [] / tls None, ObservabilityConfig::default_service_name → "aisix", default_log_level → "info", default_access_log → true, PrometheusConfig::default → { enabled: true, path: "/metrics" }, OtlpConfig::default → { enabled: false, endpoint: None }, OtlpTracingConfig::default → { enabled: false, endpoint: None, sample_ratio: 1.0 }, CacheConfig::default → { backend: Memory, redis: None }. For fields without an explicit default and with Config::validate enforcement (etcd.endpoints, proxy.addr), the column reads required. For fields whose Default is None, the column reads none.
  3. Provider-key GET response shape.crates/aisix-core/src/models/provider_key.rs:39 defines pub secret: String without #[serde(skip_serializing)], and crates/aisix-admin/src/provider_keys_handlers.rs:29-38 (get_provider_key) returns Json<ResourceEntry<ProviderKey>> — so GET /admin/v1/provider_keys/:iddoes include the plaintext secret in the response body, every time. The original draft callout in docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages #325 read "subsequent reads via GET do not include it", which was empirically incorrect. The callout in this PR was adjusted to reflect actual behavior: the warning now states that the plaintext is in both the create response and any subsequent GET, and that the same handling applies any time you read the resource.

References

Summary by CodeRabbit

  • Documentation
    • Converted narrative configuration sections into structured tables for bootstrap, observability, and cache settings (including defaults and required/optional status)
    • Clarified core concepts: model display vs upstream identifiers, API key lifecycle/rotation, and observability exporter behavior
    • Added quickstart prerequisites: upstream provider API key, Node.js 18+ for OpenAI SDK, Rust 1.93+ for self-hosted
    • Added warning to treat provider API secrets as sensitive

Review Change Stack

CopilotAI review requested due to automatic review settings May 18, 2026 04:04
@janiussyafiqjaniussyafiq added documentation Improvements or additions to documentation priority-normal labels May 18, 2026
@coderabbitai

coderabbitaiBot commented May 18, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 2f8aa0cb-5faa-4a13-a703-ebe2423802be

📥 Commits

Reviewing files that changed from the base of the PR and between 5401ff1 and 1094aac.

📒 Files selected for processing (1)
  • docs/configuration/bootstrap-config.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/configuration/bootstrap-config.md

📝 Walkthrough

Walkthrough

This PR updates documentation: restructures bootstrap configuration sections into tables with defaults and optional flags, refines core concept fields and observability exporter text, and adds explicit quickstart prerequisites plus an API-key secrecy warning.

Changes

Documentation Clarity Improvements

Layer / File(s)Summary
Bootstrap configuration tables
docs/configuration/bootstrap-config.md
etcd, proxy, admin, observability, and cache "Important fields" are converted from narrative lists into tables that show default values, optional fields (e.g., tls), reserved telemetry keys, and conditional nested blocks (e.g., redis when backend: redis).
Core concept refinements
docs/overview/core-concepts.md
Model section now documents display_name, provider, and model_name; API Key section notes plaintext bearer tokens cannot be retrieved after creation and must be rotated; Observability Exporter text focuses on OTLP-compatible telemetry export.
Quickstart prerequisites and security guidance
docs/quickstart/openai-sdk.md, docs/quickstart/self-hosted.md, docs/quickstart/first-model-first-key-first-request.md
OpenAI SDK quickstart adds a Prerequisites section (gateway + Node.js 18+); self-hosted quickstart requires Rust 1.93+ and notes first cargo run compile time; first-model guide adds upstream provider API key prerequisite and warns that admin API returns secret in plaintext and it must be treated as sensitive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This doc-only PR improves new-developer onboarding by clarifying prerequisites, model naming, credential handling, and bootstrap configuration defaults across the quickstart and overview/configuration docs.

Changes:

  • Adds Rust/Node/provider-key prerequisite guidance and build-time expectations.
  • Clarifies model alias vs upstream model ID and credential visibility.
  • Expands bootstrap configuration tables, especially observability and cache fields.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
FileDescription
docs/quickstart/self-hosted.mdAdds Rust/cargo prerequisite and first-build duration note.
docs/quickstart/openai-sdk.mdAdds gateway and Node.js/npm prerequisites.
docs/quickstart/first-model-first-key-first-request.mdClarifies provider API key prerequisite and plaintext provider-key handling.
docs/overview/core-concepts.mdClarifies model fields, API-key plaintext handling, and observability exporter description.
docs/configuration/bootstrap-config.mdAdds defaults and expanded observability/cache bootstrap field tables.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threaddocs/overview/core-concepts.md Outdated

Current data-plane behavior is based on `key_hash`, not plaintext storage. The proxy hashes the incoming bearer token and resolves it against the stored `key_hash`.

This means you cannot retrieve the plaintext bearer after creation — capture the value returned at create time, and if you lose it, use `POST /admin/v1/apikeys/:id/rotate` to issue a new one.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Rewrote the appended paragraph to reflect actual behavior: the plaintext bearer is chosen (or generated) by the caller and SHA-256-hashed locally before submission, the gateway never sees or returns the plaintext at create time, and POST /admin/v1/apikeys/:id/rotate is the only endpoint that emits a server-generated plaintext (verified against apikeys_handlers.rs:97-104 for create and :155-178 for rotate). Pushed in commit 5401ff1.

Comment on lines +161 to +164
| `metrics.otlp.enabled` | push-style OTLP metrics exporter | `false` |
| `metrics.otlp.endpoint` | OTLP/gRPC collector endpoint, e.g. `"http://otel-collector:4317"` | none |
| `tracing.otlp.enabled` | push-style OTLP traces exporter | `false` |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces | none |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Confirmed against code: install_otlp_tracer (crates/aisix-obs/src/otlp.rs:26-58) validates the endpoint and emits a startup log line but defers the real exporter pipeline ("The real exporter wires up in a follow-up PR" per the inline code comment); no install_otlp_metrics function exists in the workspace. The bootstrap-config observability table now carries a Status column marking metrics.otlp.* as reserved (not yet wired) and tracing.otlp.* as partial (validation only). Pushed in commit 5401ff1.

Comment threaddocs/overview/core-concepts.md Outdated
An `Observability Exporter` is a resource that configures external telemetry export from the gateway.

Use this concept when documenting external metrics, traces, or event forwarding behavior.
An `Observability Exporter` ships telemetry (metrics, traces, request logs) from the gateway to an external backend over an OTLP-compatible endpoint (Grafana Tempo / Loki, Honeycomb, Langfuse via OTLP, and so on). Configure one when you want gateway request and response data forwarded to your existing observability stack.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against otlp_http_sink.rs:184/:249 (the exporter builds OTLP traces payloads from UsageEvent records — per-request spans, not general metrics or logs). Rewrote the section to: "ships per-request span telemetry — derived from gateway UsageEvent records — to an OTLP/HTTP-compatible backend". Pushed in commit 5401ff1.

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | scrape-style Prometheus exporter | `true` |
| `metrics.prometheus.path` | HTTP path the Prometheus exporter is served on | `"/metrics"` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-admin/src/lib.rs:68 (/metrics route is hardcoded with no conditional gating). The bootstrap-config observability table now marks metrics.prometheus.enabled and metrics.prometheus.path as reserved (not yet consulted), with a footnote that the Prometheus exporter is currently mounted unconditionally at the hardcoded admin /metrics path. Pushed in commit 5401ff1.

| --- | --- | --- |
| `service_name` | service-name attribute attached to every metric, log, and span emitted by the process | `"aisix"` |
| `log_level` | minimum log level (`error` / `warn` / `info` / `debug` / `trace`) | `"info"` |
| `access_log` | emit a structured access-log line for every proxy request | `true` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-obs/src/access_log.rs:32-50 (AccessLog::emit() always calls tracing::info! with no config gate) and the unconditional emit_access_log call sites in every proxy handler. The bootstrap-config observability table now marks access_log as reserved (not yet consulted), with a footnote that access logs are emitted unconditionally by every proxy handler today. Pushed in commit 5401ff1.

…lity-config claims to match runtime (Copilot review)
…s fields
PR #331 (commit e6125b6) wired the `metrics.prometheus.enabled` and
`metrics.prometheus.path` consumers in `crates/aisix-admin/src/lib.rs:154-159`
plus the `normalized_prometheus_path` helper at `:164-174`, and exercised
both paths end-to-end in three new unit tests
(`metrics_endpoint_uses_configured_path`,
`metrics_endpoint_normalizes_configured_path`,
`metrics_endpoint_can_be_disabled`).
The two rows in this PR's observability field table that called those
fields "reserved (not yet consulted)" are now factually stale. Update
both rows to `wired` and align the row descriptions with #331's
runtime behaviour. Also adjust the introductory paragraph above the
table so it no longer asserts that only `service_name` and `log_level`
are consulted at runtime.
This is the consumer-trace discipline locked in during round 2 of this
PR, applied prospectively to a moving target: PR #331 changed the
consumer site after round 2's verification window, so the doc text
needs to update.
CopilotAI review requested due to automatic review settings May 18, 2026 15:24

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

docs/configuration/bootstrap-config.md:164

  • The tracing endpoint is described as validated at boot, but the implementation only checks that tracing.otlp.enabled=true has a non-empty endpoint; it does not parse or validate the endpoint URL/protocol before returning a handle. Calling this "validated" overstates the current behavior.
| `tracing.otlp.enabled` | enabling this validates the endpoint at boot and emits a startup log line; the OTLP traces pipeline itself is deferred to a future release | `false` | partial (validation only) |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces; validated at boot when `tracing.otlp.enabled` is `true` | none | partial (validation only) |

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | controls whether the admin listener mounts the Prometheus scrape endpoint; when `false`, no `/metrics` route is registered | `true` | wired |
| `metrics.prometheus.path` | mount path for the Prometheus scrape endpoint when `metrics.prometheus.enabled` is `true`; values without a leading slash are normalised by prepending one, and an empty value falls back to `/metrics` | `"/metrics"` | wired |
## `observability`

Use `observability` to configure:
Use `observability` to set process-wide telemetry knobs: service name, log level, Prometheus exporter control, and (in future releases) access-log gating and OTLP exporters. Today `service_name`, `log_level`, and the `metrics.prometheus.*` block are consulted at runtime; the remaining keys are recognized in the schema and reserved for upcoming releases — setting them is harmless but currently has no effect.
@moonming
moonming merged commit e12155b into mainMay 19, 2026
11 checks passed
janiussyafiq added a commit that referenced this pull request May 20, 2026
Two related items folded into a single revision commit per operator
authorization (msg `a5c9dec7`):
1. Merge conflict resolution against `main` post-PR-#326
PR #326 (merge commit `e12155b`, landed 2026-05-19 02:30:04Z) updated
the `## Prerequisites` block and the `## Step 3: Start the gateway`
section of `docs/quickstart/self-hosted.md` on `main`. This branch's
prior edits (etcd glossary link, `config.yaml` location anchor,
`YOUR_ADMIN_KEY` placeholder format note, "Keep the gateway running"
framing) overlapped the Prerequisites bullet on the same physical
line.
Resolution per Umar's approved Option 2: single new commit on top of
`a109d2d` that re-applies this branch's Bucket-B edits on top of the
post-#326 canonical lines:
- Add the post-#326 Rust 1.93 prerequisite bullet.
- Capitalize "A reachable etcd instance" (per #326) while preserving
this branch's `[etcd](../overview/glossary.md#etcd)` glossary link.
- Add the post-#326 "first time you run this" cargo-build duration
paragraph between the `cargo run` code block and this branch's
"Keep the gateway running. Open a new terminal..." framing. The
two paragraphs now sequence as: (a) explain first-run build time,
(b) instruct reader to keep the gateway running and switch
terminals.
Other 4 overlapping files (`bootstrap-config.md`, `core-concepts.md`,
`first-model-first-key-first-request.md`, `openai-sdk.md`) auto-merge
cleanly against post-#326 main and are not touched by this commit.
2. Copilot review address — comment id `3260189475`
The reviewer flagged that the "Capture the id (jq path)" snippet on
`docs/quickstart/first-model-first-key-first-request.md` re-ran the
same `POST /admin/v1/provider_keys` as Step 1, which would either
trigger a duplicate-`display_name` rejection from the admin API or
leave the reader with two ProviderKey rows whose ids differ from the
response example shown after the first POST. Verified empirically by
reading the two POST blocks (lines 37-46 and the captured-id snippet)
against each other.
Restructure the section so the jq path is presented as a variant of
the original create, not a second POST:
- Rename the code block title to "Create and capture the id in one
shot" to make the variant relationship explicit.
- Wrap the two paths as bulleted Option A / Option B blocks. Option
A is the jq variant of the create above ("instead of the curl
shown earlier"). Option B is the jq-free path ("if you already
ran the curl above, copy the id field by eye").
- Prefix the section with an explicit "pick one, don't run both"
callout naming the duplicate-display_name rejection as the failure
mode if the reader runs both forms.
janiussyafiq added a commit that referenced this pull request May 20, 2026
Integrate origin/main (commit 2c1d485 = post-PR-#326 / #348 plus
#330 / #341 / #343 / #345 / #346) into this branch via `git merge
--squash` to clear PR #344's lingering `mergeable: dirty` state.
Conflict on `docs/quickstart/self-hosted.md` was a 3-way-merge-base
artifact: base (3596c0a) read `- a reachable etcd instance`, main
changed `a` → `A` (via #326), this branch additionally inserted the
glossary link. Both changes are wanted; resolution per Umar's
approved plan was `git checkout --ours`, which preserves the branch's
self-hosted.md state (already integrates capital A + glossary link
+ first-time-build paragraph + keep-running framing). Other 4
overlapping doc files auto-merged cleanly (`bootstrap-config.md`,
`core-concepts.md`, `first-model-first-key-first-request.md`,
`openai-sdk.md`). Code files all auto-merged cleanly.
Additional Copilot review (post-`167196a` cycle) addressed:
- `docs/index.md:7` — change link display text from `[data-plane]`
to `[data plane]` to match the canonical glossary term. The URL
anchor `#data-plane` stays kebab-case (matches the glossary
heading's auto-anchor); only the display text changes. Comment
id 3271145422.
- `docs/quickstart/openai-sdk.md:43` — change `All three steps below`
to `All commands below`. The Install-the-SDK section has two
command blocks (mkdir+cd, npm install), not three; the prior
wording originated from a mental model (mkdir, cd, install)
that doesn't match the typographic count of code blocks under
the heading. Comment id 3271145458.
Copilot's third comment on `docs/overview/core-concepts.md`
Observability Exporter wording (id 3271145444) auto-resolves via
this merge — main's #326 rewrite supersedes the branch's pre-#326
wording at that location ("ships per-request span telemetry…
OTLP/HTTP-compatible backend…" replaces "Use this concept when
documenting…"). No separate edit needed; the merge IS the fix.
janiussyafiq added a commit that referenced this pull request May 20, 2026
…ickstart-polish
Resolve PR #344's lingering mergeable: dirty state by linking the
branch history to origin/main (2c1d485 = post-#326 / #348 / #330 /
#341 / #343 / #345 / #346).
The squash-merge commit landed earlier (e2af197) integrated main's
content into the branch tree but did not link the histories, so
GitHub's mergeable computation still saw the 3-way-merge-base
artifact conflict on docs/quickstart/self-hosted.md (a vs A + the
glossary link / "In another terminal" vs "Keep the gateway running"
framing). This explicit merge commit ties the branch to main's
history.
Self-hosted.md conflict resolved by taking OUR side — the branch's
edits already contain main's substantive changes (capital A,
first-time-build paragraph) plus this PR's additions (glossary
link, keep-running framing, YOUR_ADMIN_KEY note, config.yaml
location anchor).
The auto-merge of first-model-first-key-first-request.md duplicated
the :::warning callout that was already integrated via the squash
commit; removed the duplicate.
moonming pushed a commit that referenced this pull request May 22, 2026
@jarvis9443
jarvis9443 deleted the docs/issue-325-onboarding-flow-fixes branch June 25, 2026 06:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationpriority-normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages

3 participants

@janiussyafiq@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages - #326

Merged
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes
May 19, 2026
Merged

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages#326
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes

Conversation

@janiussyafiq

@janiussyafiqjaniussyafiq commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes six new-developer onboarding gaps surfaced by an end-to-end walkthrough of the 7-page onboarding flow (docs/index.mddocs/configuration/bootstrap-config.md) against main at 1a744ee. Three of the fixes (#1, #2, #3) close hard blockers on a clean machine (missing cargo / npm / provider-API-key prerequisites; model_not_found from display_name/model_name ambiguity; an Observability Exporter section whose second sentence is style-guide advice rather than user content). The remaining three (#4, #5, #6) close friction points along the same path (bootstrap-config.md field tables missing a Default column and an observability subsection; two credential-handling operational callouts; a cold-build duration note).

Doc-only diff. No code, schemas, configs, or test fixtures touched.

Closes#325.

Changes

FileChange
docs/quickstart/self-hosted.mdPrerequisites: add Rust 1.93+ with cargo (via rustup) as the leading prerequisite. After cargo run …: add a one-sentence note that the first-time build typically takes 3–5 minutes and that subsequent runs are incremental.
docs/quickstart/first-model-first-key-first-request.mdPrerequisites: replace "A reachable upstream OpenAI-compatible endpoint" with explicit credential-acquisition guidance (supported providers, sign-up link with cost order-of-magnitude, sk-... format hint, placeholder pointer). After the ProviderKey create-time response example: add a :::warning callout that the secret field is plaintext and that GET /admin/v1/provider_keys/:id also returns the plaintext.
docs/quickstart/openai-sdk.mdNew ## Prerequisites section placed between the "If you have not done that yet…" intro and the first ## What Changes In The SDK section. Lists a running gateway with provider key / model / API key already created, and Node.js 18+ with npm (the OpenAI SDK floor).
docs/overview/core-concepts.mdModel field-list bullet: inline-annotate display_name as the alias callers send in the API request's model field and model_name as the upstream model ID forwarded to the provider. API Key section: append a sentence after the key_hash description noting the plaintext is unrecoverable post-create and pointing at POST /admin/v1/apikeys/:id/rotate. Observability Exporter section: replace the documentation-author-facing second sentence with a user-facing description of what the resource does (OTLP-compatible export to external backends) and when to configure one.
docs/configuration/bootstrap-config.mdAdd a Default column to the field tables on etcd, proxy, admin. Build out the observability section with a new field table covering service_name, log_level, access_log, metrics.prometheus.*, metrics.otlp.*, and tracing.otlp.*, plus a pointer to the dynamic Observability Exporters page. Build out the cache section with a small field table covering backend and redis.

Net diff: 5 files, +58 / -35. Source: git diff --stat.

Test plan

Doc-only diff — no .rs files, schemas, configs, or test fixtures touched. The cargo trio was still run end-to-end as the canonical pre-merge gate.

  • cargo fmt --check — PASS (exit 0).
  • cargo clippy --workspace --all-targets -- -D warnings — PASS (exit 0; full tail captured below).
  • cargo test --workspace — PASS (exit 0; full crate-suite results captured below).
  • grep -rln <each affected page> tests/e2e/ for all 5 pages returns empty. pnpm test under tests/e2e/ is not applicable for this diff.

cargo clippy tail

 Checking aisix-proxy v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-proxy)
Checking aisix-admin v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-admin)
Checking aisix-server v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-server)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 40.28s

cargo test summary

33 test-suite-result lines, all 'ok'. Aggregate: 1029 tests passed, 0 failed, 3 ignored across the workspace (unit suites + doc-tests). Sample tail (last 3 named tests + last result line):
test keyword::tests::invalid_regex_is_a_clean_error_not_a_panic ... ok
test usage::tests::full_channel_drop_does_not_panic ... ok
test dispatch::tests::build_v1_url_rejects_path_without_leading_slash - should panic ... ok
test result: ok. <N> passed; 0 failed; <N> ignored; 0 measured; 0 filtered out

Affected pages

  • docs/quickstart/self-hosted.md
  • docs/quickstart/first-model-first-key-first-request.md
  • docs/quickstart/openai-sdk.md
  • docs/overview/core-concepts.md
  • docs/configuration/bootstrap-config.md

Pre-merge-check verification log

The three pre-merge checks called out in #325 were resolved as follows:

  1. Rust / Node minimum versions. Verified Rust against rust-toolchain.toml (channel = "1.93.1") and Cargo.toml (rust-version = "1.93"). The issue body's tentative "Rust 1.79+" floor was bumped to Rust 1.93 or newer in the doc, and a note added that rustup will pull the right channel automatically because rust-toolchain.toml pins it. For Node.js, neither tests/e2e/package.json nor a top-level package.json declares an engines floor, so the doc cites Node.js 18 or newer (the OpenAI SDK's published floor at version 4.x shipped in tests/e2e/package.json).
  2. Bootstrap-config Default column. Pulled all default values from crates/aisix-core/src/config.rs (this repo's config crate lives in aisix-core, not in a separate aisix-config crate as the issue body tentatively listed). Values pulled: EtcdConfig::default_prefix → "/aisix", default_dial_timeout / default_request_timeout → 5000ms, ProxyConfig::default_body_limit → 10 * 1024 * 1024, AdminConfig::default → addr "127.0.0.1:0" / admin_keys [] / tls None, ObservabilityConfig::default_service_name → "aisix", default_log_level → "info", default_access_log → true, PrometheusConfig::default → { enabled: true, path: "/metrics" }, OtlpConfig::default → { enabled: false, endpoint: None }, OtlpTracingConfig::default → { enabled: false, endpoint: None, sample_ratio: 1.0 }, CacheConfig::default → { backend: Memory, redis: None }. For fields without an explicit default and with Config::validate enforcement (etcd.endpoints, proxy.addr), the column reads required. For fields whose Default is None, the column reads none.
  3. Provider-key GET response shape.crates/aisix-core/src/models/provider_key.rs:39 defines pub secret: String without #[serde(skip_serializing)], and crates/aisix-admin/src/provider_keys_handlers.rs:29-38 (get_provider_key) returns Json<ResourceEntry<ProviderKey>> — so GET /admin/v1/provider_keys/:iddoes include the plaintext secret in the response body, every time. The original draft callout in docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages #325 read "subsequent reads via GET do not include it", which was empirically incorrect. The callout in this PR was adjusted to reflect actual behavior: the warning now states that the plaintext is in both the create response and any subsequent GET, and that the same handling applies any time you read the resource.

References

Summary by CodeRabbit

  • Documentation
    • Converted narrative configuration sections into structured tables for bootstrap, observability, and cache settings (including defaults and required/optional status)
    • Clarified core concepts: model display vs upstream identifiers, API key lifecycle/rotation, and observability exporter behavior
    • Added quickstart prerequisites: upstream provider API key, Node.js 18+ for OpenAI SDK, Rust 1.93+ for self-hosted
    • Added warning to treat provider API secrets as sensitive

Review Change Stack

CopilotAI review requested due to automatic review settings May 18, 2026 04:04
@janiussyafiqjaniussyafiq added documentation Improvements or additions to documentation priority-normal labels May 18, 2026
@coderabbitai

coderabbitaiBot commented May 18, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 2f8aa0cb-5faa-4a13-a703-ebe2423802be

📥 Commits

Reviewing files that changed from the base of the PR and between 5401ff1 and 1094aac.

📒 Files selected for processing (1)
  • docs/configuration/bootstrap-config.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/configuration/bootstrap-config.md

📝 Walkthrough

Walkthrough

This PR updates documentation: restructures bootstrap configuration sections into tables with defaults and optional flags, refines core concept fields and observability exporter text, and adds explicit quickstart prerequisites plus an API-key secrecy warning.

Changes

Documentation Clarity Improvements

Layer / File(s)Summary
Bootstrap configuration tables
docs/configuration/bootstrap-config.md
etcd, proxy, admin, observability, and cache "Important fields" are converted from narrative lists into tables that show default values, optional fields (e.g., tls), reserved telemetry keys, and conditional nested blocks (e.g., redis when backend: redis).
Core concept refinements
docs/overview/core-concepts.md
Model section now documents display_name, provider, and model_name; API Key section notes plaintext bearer tokens cannot be retrieved after creation and must be rotated; Observability Exporter text focuses on OTLP-compatible telemetry export.
Quickstart prerequisites and security guidance
docs/quickstart/openai-sdk.md, docs/quickstart/self-hosted.md, docs/quickstart/first-model-first-key-first-request.md
OpenAI SDK quickstart adds a Prerequisites section (gateway + Node.js 18+); self-hosted quickstart requires Rust 1.93+ and notes first cargo run compile time; first-model guide adds upstream provider API key prerequisite and warns that admin API returns secret in plaintext and it must be treated as sensitive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This doc-only PR improves new-developer onboarding by clarifying prerequisites, model naming, credential handling, and bootstrap configuration defaults across the quickstart and overview/configuration docs.

Changes:

  • Adds Rust/Node/provider-key prerequisite guidance and build-time expectations.
  • Clarifies model alias vs upstream model ID and credential visibility.
  • Expands bootstrap configuration tables, especially observability and cache fields.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
FileDescription
docs/quickstart/self-hosted.mdAdds Rust/cargo prerequisite and first-build duration note.
docs/quickstart/openai-sdk.mdAdds gateway and Node.js/npm prerequisites.
docs/quickstart/first-model-first-key-first-request.mdClarifies provider API key prerequisite and plaintext provider-key handling.
docs/overview/core-concepts.mdClarifies model fields, API-key plaintext handling, and observability exporter description.
docs/configuration/bootstrap-config.mdAdds defaults and expanded observability/cache bootstrap field tables.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threaddocs/overview/core-concepts.md Outdated

Current data-plane behavior is based on `key_hash`, not plaintext storage. The proxy hashes the incoming bearer token and resolves it against the stored `key_hash`.

This means you cannot retrieve the plaintext bearer after creation — capture the value returned at create time, and if you lose it, use `POST /admin/v1/apikeys/:id/rotate` to issue a new one.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Rewrote the appended paragraph to reflect actual behavior: the plaintext bearer is chosen (or generated) by the caller and SHA-256-hashed locally before submission, the gateway never sees or returns the plaintext at create time, and POST /admin/v1/apikeys/:id/rotate is the only endpoint that emits a server-generated plaintext (verified against apikeys_handlers.rs:97-104 for create and :155-178 for rotate). Pushed in commit 5401ff1.

Comment on lines +161 to +164
| `metrics.otlp.enabled` | push-style OTLP metrics exporter | `false` |
| `metrics.otlp.endpoint` | OTLP/gRPC collector endpoint, e.g. `"http://otel-collector:4317"` | none |
| `tracing.otlp.enabled` | push-style OTLP traces exporter | `false` |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces | none |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Confirmed against code: install_otlp_tracer (crates/aisix-obs/src/otlp.rs:26-58) validates the endpoint and emits a startup log line but defers the real exporter pipeline ("The real exporter wires up in a follow-up PR" per the inline code comment); no install_otlp_metrics function exists in the workspace. The bootstrap-config observability table now carries a Status column marking metrics.otlp.* as reserved (not yet wired) and tracing.otlp.* as partial (validation only). Pushed in commit 5401ff1.

Comment threaddocs/overview/core-concepts.md Outdated
An `Observability Exporter` is a resource that configures external telemetry export from the gateway.

Use this concept when documenting external metrics, traces, or event forwarding behavior.
An `Observability Exporter` ships telemetry (metrics, traces, request logs) from the gateway to an external backend over an OTLP-compatible endpoint (Grafana Tempo / Loki, Honeycomb, Langfuse via OTLP, and so on). Configure one when you want gateway request and response data forwarded to your existing observability stack.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against otlp_http_sink.rs:184/:249 (the exporter builds OTLP traces payloads from UsageEvent records — per-request spans, not general metrics or logs). Rewrote the section to: "ships per-request span telemetry — derived from gateway UsageEvent records — to an OTLP/HTTP-compatible backend". Pushed in commit 5401ff1.

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | scrape-style Prometheus exporter | `true` |
| `metrics.prometheus.path` | HTTP path the Prometheus exporter is served on | `"/metrics"` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-admin/src/lib.rs:68 (/metrics route is hardcoded with no conditional gating). The bootstrap-config observability table now marks metrics.prometheus.enabled and metrics.prometheus.path as reserved (not yet consulted), with a footnote that the Prometheus exporter is currently mounted unconditionally at the hardcoded admin /metrics path. Pushed in commit 5401ff1.

| --- | --- | --- |
| `service_name` | service-name attribute attached to every metric, log, and span emitted by the process | `"aisix"` |
| `log_level` | minimum log level (`error` / `warn` / `info` / `debug` / `trace`) | `"info"` |
| `access_log` | emit a structured access-log line for every proxy request | `true` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-obs/src/access_log.rs:32-50 (AccessLog::emit() always calls tracing::info! with no config gate) and the unconditional emit_access_log call sites in every proxy handler. The bootstrap-config observability table now marks access_log as reserved (not yet consulted), with a footnote that access logs are emitted unconditionally by every proxy handler today. Pushed in commit 5401ff1.

…lity-config claims to match runtime (Copilot review)
…s fields
PR #331 (commit e6125b6) wired the `metrics.prometheus.enabled` and
`metrics.prometheus.path` consumers in `crates/aisix-admin/src/lib.rs:154-159`
plus the `normalized_prometheus_path` helper at `:164-174`, and exercised
both paths end-to-end in three new unit tests
(`metrics_endpoint_uses_configured_path`,
`metrics_endpoint_normalizes_configured_path`,
`metrics_endpoint_can_be_disabled`).
The two rows in this PR's observability field table that called those
fields "reserved (not yet consulted)" are now factually stale. Update
both rows to `wired` and align the row descriptions with #331's
runtime behaviour. Also adjust the introductory paragraph above the
table so it no longer asserts that only `service_name` and `log_level`
are consulted at runtime.
This is the consumer-trace discipline locked in during round 2 of this
PR, applied prospectively to a moving target: PR #331 changed the
consumer site after round 2's verification window, so the doc text
needs to update.
CopilotAI review requested due to automatic review settings May 18, 2026 15:24

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

docs/configuration/bootstrap-config.md:164

  • The tracing endpoint is described as validated at boot, but the implementation only checks that tracing.otlp.enabled=true has a non-empty endpoint; it does not parse or validate the endpoint URL/protocol before returning a handle. Calling this "validated" overstates the current behavior.
| `tracing.otlp.enabled` | enabling this validates the endpoint at boot and emits a startup log line; the OTLP traces pipeline itself is deferred to a future release | `false` | partial (validation only) |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces; validated at boot when `tracing.otlp.enabled` is `true` | none | partial (validation only) |

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | controls whether the admin listener mounts the Prometheus scrape endpoint; when `false`, no `/metrics` route is registered | `true` | wired |
| `metrics.prometheus.path` | mount path for the Prometheus scrape endpoint when `metrics.prometheus.enabled` is `true`; values without a leading slash are normalised by prepending one, and an empty value falls back to `/metrics` | `"/metrics"` | wired |
## `observability`

Use `observability` to configure:
Use `observability` to set process-wide telemetry knobs: service name, log level, Prometheus exporter control, and (in future releases) access-log gating and OTLP exporters. Today `service_name`, `log_level`, and the `metrics.prometheus.*` block are consulted at runtime; the remaining keys are recognized in the schema and reserved for upcoming releases — setting them is harmless but currently has no effect.
@moonming
moonming merged commit e12155b into mainMay 19, 2026
11 checks passed
janiussyafiq added a commit that referenced this pull request May 20, 2026
Two related items folded into a single revision commit per operator
authorization (msg `a5c9dec7`):
1. Merge conflict resolution against `main` post-PR-#326
PR #326 (merge commit `e12155b`, landed 2026-05-19 02:30:04Z) updated
the `## Prerequisites` block and the `## Step 3: Start the gateway`
section of `docs/quickstart/self-hosted.md` on `main`. This branch's
prior edits (etcd glossary link, `config.yaml` location anchor,
`YOUR_ADMIN_KEY` placeholder format note, "Keep the gateway running"
framing) overlapped the Prerequisites bullet on the same physical
line.
Resolution per Umar's approved Option 2: single new commit on top of
`a109d2d` that re-applies this branch's Bucket-B edits on top of the
post-#326 canonical lines:
- Add the post-#326 Rust 1.93 prerequisite bullet.
- Capitalize "A reachable etcd instance" (per #326) while preserving
this branch's `[etcd](../overview/glossary.md#etcd)` glossary link.
- Add the post-#326 "first time you run this" cargo-build duration
paragraph between the `cargo run` code block and this branch's
"Keep the gateway running. Open a new terminal..." framing. The
two paragraphs now sequence as: (a) explain first-run build time,
(b) instruct reader to keep the gateway running and switch
terminals.
Other 4 overlapping files (`bootstrap-config.md`, `core-concepts.md`,
`first-model-first-key-first-request.md`, `openai-sdk.md`) auto-merge
cleanly against post-#326 main and are not touched by this commit.
2. Copilot review address — comment id `3260189475`
The reviewer flagged that the "Capture the id (jq path)" snippet on
`docs/quickstart/first-model-first-key-first-request.md` re-ran the
same `POST /admin/v1/provider_keys` as Step 1, which would either
trigger a duplicate-`display_name` rejection from the admin API or
leave the reader with two ProviderKey rows whose ids differ from the
response example shown after the first POST. Verified empirically by
reading the two POST blocks (lines 37-46 and the captured-id snippet)
against each other.
Restructure the section so the jq path is presented as a variant of
the original create, not a second POST:
- Rename the code block title to "Create and capture the id in one
shot" to make the variant relationship explicit.
- Wrap the two paths as bulleted Option A / Option B blocks. Option
A is the jq variant of the create above ("instead of the curl
shown earlier"). Option B is the jq-free path ("if you already
ran the curl above, copy the id field by eye").
- Prefix the section with an explicit "pick one, don't run both"
callout naming the duplicate-display_name rejection as the failure
mode if the reader runs both forms.
janiussyafiq added a commit that referenced this pull request May 20, 2026
Integrate origin/main (commit 2c1d485 = post-PR-#326 / #348 plus
#330 / #341 / #343 / #345 / #346) into this branch via `git merge
--squash` to clear PR #344's lingering `mergeable: dirty` state.
Conflict on `docs/quickstart/self-hosted.md` was a 3-way-merge-base
artifact: base (3596c0a) read `- a reachable etcd instance`, main
changed `a` → `A` (via #326), this branch additionally inserted the
glossary link. Both changes are wanted; resolution per Umar's
approved plan was `git checkout --ours`, which preserves the branch's
self-hosted.md state (already integrates capital A + glossary link
+ first-time-build paragraph + keep-running framing). Other 4
overlapping doc files auto-merged cleanly (`bootstrap-config.md`,
`core-concepts.md`, `first-model-first-key-first-request.md`,
`openai-sdk.md`). Code files all auto-merged cleanly.
Additional Copilot review (post-`167196a` cycle) addressed:
- `docs/index.md:7` — change link display text from `[data-plane]`
to `[data plane]` to match the canonical glossary term. The URL
anchor `#data-plane` stays kebab-case (matches the glossary
heading's auto-anchor); only the display text changes. Comment
id 3271145422.
- `docs/quickstart/openai-sdk.md:43` — change `All three steps below`
to `All commands below`. The Install-the-SDK section has two
command blocks (mkdir+cd, npm install), not three; the prior
wording originated from a mental model (mkdir, cd, install)
that doesn't match the typographic count of code blocks under
the heading. Comment id 3271145458.
Copilot's third comment on `docs/overview/core-concepts.md`
Observability Exporter wording (id 3271145444) auto-resolves via
this merge — main's #326 rewrite supersedes the branch's pre-#326
wording at that location ("ships per-request span telemetry…
OTLP/HTTP-compatible backend…" replaces "Use this concept when
documenting…"). No separate edit needed; the merge IS the fix.
janiussyafiq added a commit that referenced this pull request May 20, 2026
…ickstart-polish
Resolve PR #344's lingering mergeable: dirty state by linking the
branch history to origin/main (2c1d485 = post-#326 / #348 / #330 /
#341 / #343 / #345 / #346).
The squash-merge commit landed earlier (e2af197) integrated main's
content into the branch tree but did not link the histories, so
GitHub's mergeable computation still saw the 3-way-merge-base
artifact conflict on docs/quickstart/self-hosted.md (a vs A + the
glossary link / "In another terminal" vs "Keep the gateway running"
framing). This explicit merge commit ties the branch to main's
history.
Self-hosted.md conflict resolved by taking OUR side — the branch's
edits already contain main's substantive changes (capital A,
first-time-build paragraph) plus this PR's additions (glossary
link, keep-running framing, YOUR_ADMIN_KEY note, config.yaml
location anchor).
The auto-merge of first-model-first-key-first-request.md duplicated
the :::warning callout that was already integrated via the squash
commit; removed the duplicate.
moonming pushed a commit that referenced this pull request May 22, 2026
@jarvis9443
jarvis9443 deleted the docs/issue-325-onboarding-flow-fixes branch June 25, 2026 06:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationpriority-normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages

3 participants

@janiussyafiq@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages - #326

Merged
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes
May 19, 2026
Merged

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages#326
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes

Conversation

@janiussyafiq

@janiussyafiqjaniussyafiq commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes six new-developer onboarding gaps surfaced by an end-to-end walkthrough of the 7-page onboarding flow (docs/index.mddocs/configuration/bootstrap-config.md) against main at 1a744ee. Three of the fixes (#1, #2, #3) close hard blockers on a clean machine (missing cargo / npm / provider-API-key prerequisites; model_not_found from display_name/model_name ambiguity; an Observability Exporter section whose second sentence is style-guide advice rather than user content). The remaining three (#4, #5, #6) close friction points along the same path (bootstrap-config.md field tables missing a Default column and an observability subsection; two credential-handling operational callouts; a cold-build duration note).

Doc-only diff. No code, schemas, configs, or test fixtures touched.

Closes#325.

Changes

FileChange
docs/quickstart/self-hosted.mdPrerequisites: add Rust 1.93+ with cargo (via rustup) as the leading prerequisite. After cargo run …: add a one-sentence note that the first-time build typically takes 3–5 minutes and that subsequent runs are incremental.
docs/quickstart/first-model-first-key-first-request.mdPrerequisites: replace "A reachable upstream OpenAI-compatible endpoint" with explicit credential-acquisition guidance (supported providers, sign-up link with cost order-of-magnitude, sk-... format hint, placeholder pointer). After the ProviderKey create-time response example: add a :::warning callout that the secret field is plaintext and that GET /admin/v1/provider_keys/:id also returns the plaintext.
docs/quickstart/openai-sdk.mdNew ## Prerequisites section placed between the "If you have not done that yet…" intro and the first ## What Changes In The SDK section. Lists a running gateway with provider key / model / API key already created, and Node.js 18+ with npm (the OpenAI SDK floor).
docs/overview/core-concepts.mdModel field-list bullet: inline-annotate display_name as the alias callers send in the API request's model field and model_name as the upstream model ID forwarded to the provider. API Key section: append a sentence after the key_hash description noting the plaintext is unrecoverable post-create and pointing at POST /admin/v1/apikeys/:id/rotate. Observability Exporter section: replace the documentation-author-facing second sentence with a user-facing description of what the resource does (OTLP-compatible export to external backends) and when to configure one.
docs/configuration/bootstrap-config.mdAdd a Default column to the field tables on etcd, proxy, admin. Build out the observability section with a new field table covering service_name, log_level, access_log, metrics.prometheus.*, metrics.otlp.*, and tracing.otlp.*, plus a pointer to the dynamic Observability Exporters page. Build out the cache section with a small field table covering backend and redis.

Net diff: 5 files, +58 / -35. Source: git diff --stat.

Test plan

Doc-only diff — no .rs files, schemas, configs, or test fixtures touched. The cargo trio was still run end-to-end as the canonical pre-merge gate.

  • cargo fmt --check — PASS (exit 0).
  • cargo clippy --workspace --all-targets -- -D warnings — PASS (exit 0; full tail captured below).
  • cargo test --workspace — PASS (exit 0; full crate-suite results captured below).
  • grep -rln <each affected page> tests/e2e/ for all 5 pages returns empty. pnpm test under tests/e2e/ is not applicable for this diff.

cargo clippy tail

 Checking aisix-proxy v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-proxy)
Checking aisix-admin v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-admin)
Checking aisix-server v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-server)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 40.28s

cargo test summary

33 test-suite-result lines, all 'ok'. Aggregate: 1029 tests passed, 0 failed, 3 ignored across the workspace (unit suites + doc-tests). Sample tail (last 3 named tests + last result line):
test keyword::tests::invalid_regex_is_a_clean_error_not_a_panic ... ok
test usage::tests::full_channel_drop_does_not_panic ... ok
test dispatch::tests::build_v1_url_rejects_path_without_leading_slash - should panic ... ok
test result: ok. <N> passed; 0 failed; <N> ignored; 0 measured; 0 filtered out

Affected pages

  • docs/quickstart/self-hosted.md
  • docs/quickstart/first-model-first-key-first-request.md
  • docs/quickstart/openai-sdk.md
  • docs/overview/core-concepts.md
  • docs/configuration/bootstrap-config.md

Pre-merge-check verification log

The three pre-merge checks called out in #325 were resolved as follows:

  1. Rust / Node minimum versions. Verified Rust against rust-toolchain.toml (channel = "1.93.1") and Cargo.toml (rust-version = "1.93"). The issue body's tentative "Rust 1.79+" floor was bumped to Rust 1.93 or newer in the doc, and a note added that rustup will pull the right channel automatically because rust-toolchain.toml pins it. For Node.js, neither tests/e2e/package.json nor a top-level package.json declares an engines floor, so the doc cites Node.js 18 or newer (the OpenAI SDK's published floor at version 4.x shipped in tests/e2e/package.json).
  2. Bootstrap-config Default column. Pulled all default values from crates/aisix-core/src/config.rs (this repo's config crate lives in aisix-core, not in a separate aisix-config crate as the issue body tentatively listed). Values pulled: EtcdConfig::default_prefix → "/aisix", default_dial_timeout / default_request_timeout → 5000ms, ProxyConfig::default_body_limit → 10 * 1024 * 1024, AdminConfig::default → addr "127.0.0.1:0" / admin_keys [] / tls None, ObservabilityConfig::default_service_name → "aisix", default_log_level → "info", default_access_log → true, PrometheusConfig::default → { enabled: true, path: "/metrics" }, OtlpConfig::default → { enabled: false, endpoint: None }, OtlpTracingConfig::default → { enabled: false, endpoint: None, sample_ratio: 1.0 }, CacheConfig::default → { backend: Memory, redis: None }. For fields without an explicit default and with Config::validate enforcement (etcd.endpoints, proxy.addr), the column reads required. For fields whose Default is None, the column reads none.
  3. Provider-key GET response shape.crates/aisix-core/src/models/provider_key.rs:39 defines pub secret: String without #[serde(skip_serializing)], and crates/aisix-admin/src/provider_keys_handlers.rs:29-38 (get_provider_key) returns Json<ResourceEntry<ProviderKey>> — so GET /admin/v1/provider_keys/:iddoes include the plaintext secret in the response body, every time. The original draft callout in docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages #325 read "subsequent reads via GET do not include it", which was empirically incorrect. The callout in this PR was adjusted to reflect actual behavior: the warning now states that the plaintext is in both the create response and any subsequent GET, and that the same handling applies any time you read the resource.

References

Summary by CodeRabbit

  • Documentation
    • Converted narrative configuration sections into structured tables for bootstrap, observability, and cache settings (including defaults and required/optional status)
    • Clarified core concepts: model display vs upstream identifiers, API key lifecycle/rotation, and observability exporter behavior
    • Added quickstart prerequisites: upstream provider API key, Node.js 18+ for OpenAI SDK, Rust 1.93+ for self-hosted
    • Added warning to treat provider API secrets as sensitive

Review Change Stack

CopilotAI review requested due to automatic review settings May 18, 2026 04:04
@janiussyafiqjaniussyafiq added documentation Improvements or additions to documentation priority-normal labels May 18, 2026
@coderabbitai

coderabbitaiBot commented May 18, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 2f8aa0cb-5faa-4a13-a703-ebe2423802be

📥 Commits

Reviewing files that changed from the base of the PR and between 5401ff1 and 1094aac.

📒 Files selected for processing (1)
  • docs/configuration/bootstrap-config.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/configuration/bootstrap-config.md

📝 Walkthrough

Walkthrough

This PR updates documentation: restructures bootstrap configuration sections into tables with defaults and optional flags, refines core concept fields and observability exporter text, and adds explicit quickstart prerequisites plus an API-key secrecy warning.

Changes

Documentation Clarity Improvements

Layer / File(s)Summary
Bootstrap configuration tables
docs/configuration/bootstrap-config.md
etcd, proxy, admin, observability, and cache "Important fields" are converted from narrative lists into tables that show default values, optional fields (e.g., tls), reserved telemetry keys, and conditional nested blocks (e.g., redis when backend: redis).
Core concept refinements
docs/overview/core-concepts.md
Model section now documents display_name, provider, and model_name; API Key section notes plaintext bearer tokens cannot be retrieved after creation and must be rotated; Observability Exporter text focuses on OTLP-compatible telemetry export.
Quickstart prerequisites and security guidance
docs/quickstart/openai-sdk.md, docs/quickstart/self-hosted.md, docs/quickstart/first-model-first-key-first-request.md
OpenAI SDK quickstart adds a Prerequisites section (gateway + Node.js 18+); self-hosted quickstart requires Rust 1.93+ and notes first cargo run compile time; first-model guide adds upstream provider API key prerequisite and warns that admin API returns secret in plaintext and it must be treated as sensitive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This doc-only PR improves new-developer onboarding by clarifying prerequisites, model naming, credential handling, and bootstrap configuration defaults across the quickstart and overview/configuration docs.

Changes:

  • Adds Rust/Node/provider-key prerequisite guidance and build-time expectations.
  • Clarifies model alias vs upstream model ID and credential visibility.
  • Expands bootstrap configuration tables, especially observability and cache fields.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
FileDescription
docs/quickstart/self-hosted.mdAdds Rust/cargo prerequisite and first-build duration note.
docs/quickstart/openai-sdk.mdAdds gateway and Node.js/npm prerequisites.
docs/quickstart/first-model-first-key-first-request.mdClarifies provider API key prerequisite and plaintext provider-key handling.
docs/overview/core-concepts.mdClarifies model fields, API-key plaintext handling, and observability exporter description.
docs/configuration/bootstrap-config.mdAdds defaults and expanded observability/cache bootstrap field tables.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threaddocs/overview/core-concepts.md Outdated

Current data-plane behavior is based on `key_hash`, not plaintext storage. The proxy hashes the incoming bearer token and resolves it against the stored `key_hash`.

This means you cannot retrieve the plaintext bearer after creation — capture the value returned at create time, and if you lose it, use `POST /admin/v1/apikeys/:id/rotate` to issue a new one.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Rewrote the appended paragraph to reflect actual behavior: the plaintext bearer is chosen (or generated) by the caller and SHA-256-hashed locally before submission, the gateway never sees or returns the plaintext at create time, and POST /admin/v1/apikeys/:id/rotate is the only endpoint that emits a server-generated plaintext (verified against apikeys_handlers.rs:97-104 for create and :155-178 for rotate). Pushed in commit 5401ff1.

Comment on lines +161 to +164
| `metrics.otlp.enabled` | push-style OTLP metrics exporter | `false` |
| `metrics.otlp.endpoint` | OTLP/gRPC collector endpoint, e.g. `"http://otel-collector:4317"` | none |
| `tracing.otlp.enabled` | push-style OTLP traces exporter | `false` |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces | none |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Confirmed against code: install_otlp_tracer (crates/aisix-obs/src/otlp.rs:26-58) validates the endpoint and emits a startup log line but defers the real exporter pipeline ("The real exporter wires up in a follow-up PR" per the inline code comment); no install_otlp_metrics function exists in the workspace. The bootstrap-config observability table now carries a Status column marking metrics.otlp.* as reserved (not yet wired) and tracing.otlp.* as partial (validation only). Pushed in commit 5401ff1.

Comment threaddocs/overview/core-concepts.md Outdated
An `Observability Exporter` is a resource that configures external telemetry export from the gateway.

Use this concept when documenting external metrics, traces, or event forwarding behavior.
An `Observability Exporter` ships telemetry (metrics, traces, request logs) from the gateway to an external backend over an OTLP-compatible endpoint (Grafana Tempo / Loki, Honeycomb, Langfuse via OTLP, and so on). Configure one when you want gateway request and response data forwarded to your existing observability stack.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against otlp_http_sink.rs:184/:249 (the exporter builds OTLP traces payloads from UsageEvent records — per-request spans, not general metrics or logs). Rewrote the section to: "ships per-request span telemetry — derived from gateway UsageEvent records — to an OTLP/HTTP-compatible backend". Pushed in commit 5401ff1.

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | scrape-style Prometheus exporter | `true` |
| `metrics.prometheus.path` | HTTP path the Prometheus exporter is served on | `"/metrics"` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-admin/src/lib.rs:68 (/metrics route is hardcoded with no conditional gating). The bootstrap-config observability table now marks metrics.prometheus.enabled and metrics.prometheus.path as reserved (not yet consulted), with a footnote that the Prometheus exporter is currently mounted unconditionally at the hardcoded admin /metrics path. Pushed in commit 5401ff1.

| --- | --- | --- |
| `service_name` | service-name attribute attached to every metric, log, and span emitted by the process | `"aisix"` |
| `log_level` | minimum log level (`error` / `warn` / `info` / `debug` / `trace`) | `"info"` |
| `access_log` | emit a structured access-log line for every proxy request | `true` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-obs/src/access_log.rs:32-50 (AccessLog::emit() always calls tracing::info! with no config gate) and the unconditional emit_access_log call sites in every proxy handler. The bootstrap-config observability table now marks access_log as reserved (not yet consulted), with a footnote that access logs are emitted unconditionally by every proxy handler today. Pushed in commit 5401ff1.

…lity-config claims to match runtime (Copilot review)
…s fields
PR #331 (commit e6125b6) wired the `metrics.prometheus.enabled` and
`metrics.prometheus.path` consumers in `crates/aisix-admin/src/lib.rs:154-159`
plus the `normalized_prometheus_path` helper at `:164-174`, and exercised
both paths end-to-end in three new unit tests
(`metrics_endpoint_uses_configured_path`,
`metrics_endpoint_normalizes_configured_path`,
`metrics_endpoint_can_be_disabled`).
The two rows in this PR's observability field table that called those
fields "reserved (not yet consulted)" are now factually stale. Update
both rows to `wired` and align the row descriptions with #331's
runtime behaviour. Also adjust the introductory paragraph above the
table so it no longer asserts that only `service_name` and `log_level`
are consulted at runtime.
This is the consumer-trace discipline locked in during round 2 of this
PR, applied prospectively to a moving target: PR #331 changed the
consumer site after round 2's verification window, so the doc text
needs to update.
CopilotAI review requested due to automatic review settings May 18, 2026 15:24

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

docs/configuration/bootstrap-config.md:164

  • The tracing endpoint is described as validated at boot, but the implementation only checks that tracing.otlp.enabled=true has a non-empty endpoint; it does not parse or validate the endpoint URL/protocol before returning a handle. Calling this "validated" overstates the current behavior.
| `tracing.otlp.enabled` | enabling this validates the endpoint at boot and emits a startup log line; the OTLP traces pipeline itself is deferred to a future release | `false` | partial (validation only) |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces; validated at boot when `tracing.otlp.enabled` is `true` | none | partial (validation only) |

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | controls whether the admin listener mounts the Prometheus scrape endpoint; when `false`, no `/metrics` route is registered | `true` | wired |
| `metrics.prometheus.path` | mount path for the Prometheus scrape endpoint when `metrics.prometheus.enabled` is `true`; values without a leading slash are normalised by prepending one, and an empty value falls back to `/metrics` | `"/metrics"` | wired |
## `observability`

Use `observability` to configure:
Use `observability` to set process-wide telemetry knobs: service name, log level, Prometheus exporter control, and (in future releases) access-log gating and OTLP exporters. Today `service_name`, `log_level`, and the `metrics.prometheus.*` block are consulted at runtime; the remaining keys are recognized in the schema and reserved for upcoming releases — setting them is harmless but currently has no effect.
@moonming
moonming merged commit e12155b into mainMay 19, 2026
11 checks passed
janiussyafiq added a commit that referenced this pull request May 20, 2026
Two related items folded into a single revision commit per operator
authorization (msg `a5c9dec7`):
1. Merge conflict resolution against `main` post-PR-#326
PR #326 (merge commit `e12155b`, landed 2026-05-19 02:30:04Z) updated
the `## Prerequisites` block and the `## Step 3: Start the gateway`
section of `docs/quickstart/self-hosted.md` on `main`. This branch's
prior edits (etcd glossary link, `config.yaml` location anchor,
`YOUR_ADMIN_KEY` placeholder format note, "Keep the gateway running"
framing) overlapped the Prerequisites bullet on the same physical
line.
Resolution per Umar's approved Option 2: single new commit on top of
`a109d2d` that re-applies this branch's Bucket-B edits on top of the
post-#326 canonical lines:
- Add the post-#326 Rust 1.93 prerequisite bullet.
- Capitalize "A reachable etcd instance" (per #326) while preserving
this branch's `[etcd](../overview/glossary.md#etcd)` glossary link.
- Add the post-#326 "first time you run this" cargo-build duration
paragraph between the `cargo run` code block and this branch's
"Keep the gateway running. Open a new terminal..." framing. The
two paragraphs now sequence as: (a) explain first-run build time,
(b) instruct reader to keep the gateway running and switch
terminals.
Other 4 overlapping files (`bootstrap-config.md`, `core-concepts.md`,
`first-model-first-key-first-request.md`, `openai-sdk.md`) auto-merge
cleanly against post-#326 main and are not touched by this commit.
2. Copilot review address — comment id `3260189475`
The reviewer flagged that the "Capture the id (jq path)" snippet on
`docs/quickstart/first-model-first-key-first-request.md` re-ran the
same `POST /admin/v1/provider_keys` as Step 1, which would either
trigger a duplicate-`display_name` rejection from the admin API or
leave the reader with two ProviderKey rows whose ids differ from the
response example shown after the first POST. Verified empirically by
reading the two POST blocks (lines 37-46 and the captured-id snippet)
against each other.
Restructure the section so the jq path is presented as a variant of
the original create, not a second POST:
- Rename the code block title to "Create and capture the id in one
shot" to make the variant relationship explicit.
- Wrap the two paths as bulleted Option A / Option B blocks. Option
A is the jq variant of the create above ("instead of the curl
shown earlier"). Option B is the jq-free path ("if you already
ran the curl above, copy the id field by eye").
- Prefix the section with an explicit "pick one, don't run both"
callout naming the duplicate-display_name rejection as the failure
mode if the reader runs both forms.
janiussyafiq added a commit that referenced this pull request May 20, 2026
Integrate origin/main (commit 2c1d485 = post-PR-#326 / #348 plus
#330 / #341 / #343 / #345 / #346) into this branch via `git merge
--squash` to clear PR #344's lingering `mergeable: dirty` state.
Conflict on `docs/quickstart/self-hosted.md` was a 3-way-merge-base
artifact: base (3596c0a) read `- a reachable etcd instance`, main
changed `a` → `A` (via #326), this branch additionally inserted the
glossary link. Both changes are wanted; resolution per Umar's
approved plan was `git checkout --ours`, which preserves the branch's
self-hosted.md state (already integrates capital A + glossary link
+ first-time-build paragraph + keep-running framing). Other 4
overlapping doc files auto-merged cleanly (`bootstrap-config.md`,
`core-concepts.md`, `first-model-first-key-first-request.md`,
`openai-sdk.md`). Code files all auto-merged cleanly.
Additional Copilot review (post-`167196a` cycle) addressed:
- `docs/index.md:7` — change link display text from `[data-plane]`
to `[data plane]` to match the canonical glossary term. The URL
anchor `#data-plane` stays kebab-case (matches the glossary
heading's auto-anchor); only the display text changes. Comment
id 3271145422.
- `docs/quickstart/openai-sdk.md:43` — change `All three steps below`
to `All commands below`. The Install-the-SDK section has two
command blocks (mkdir+cd, npm install), not three; the prior
wording originated from a mental model (mkdir, cd, install)
that doesn't match the typographic count of code blocks under
the heading. Comment id 3271145458.
Copilot's third comment on `docs/overview/core-concepts.md`
Observability Exporter wording (id 3271145444) auto-resolves via
this merge — main's #326 rewrite supersedes the branch's pre-#326
wording at that location ("ships per-request span telemetry…
OTLP/HTTP-compatible backend…" replaces "Use this concept when
documenting…"). No separate edit needed; the merge IS the fix.
janiussyafiq added a commit that referenced this pull request May 20, 2026
…ickstart-polish
Resolve PR #344's lingering mergeable: dirty state by linking the
branch history to origin/main (2c1d485 = post-#326 / #348 / #330 /
#341 / #343 / #345 / #346).
The squash-merge commit landed earlier (e2af197) integrated main's
content into the branch tree but did not link the histories, so
GitHub's mergeable computation still saw the 3-way-merge-base
artifact conflict on docs/quickstart/self-hosted.md (a vs A + the
glossary link / "In another terminal" vs "Keep the gateway running"
framing). This explicit merge commit ties the branch to main's
history.
Self-hosted.md conflict resolved by taking OUR side — the branch's
edits already contain main's substantive changes (capital A,
first-time-build paragraph) plus this PR's additions (glossary
link, keep-running framing, YOUR_ADMIN_KEY note, config.yaml
location anchor).
The auto-merge of first-model-first-key-first-request.md duplicated
the :::warning callout that was already integrated via the squash
commit; removed the duplicate.
moonming pushed a commit that referenced this pull request May 22, 2026
@jarvis9443
jarvis9443 deleted the docs/issue-325-onboarding-flow-fixes branch June 25, 2026 06:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationpriority-normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages

3 participants

@janiussyafiq@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages - #326

Merged
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes
May 19, 2026
Merged

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages#326
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes

Conversation

@janiussyafiq

@janiussyafiqjaniussyafiq commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes six new-developer onboarding gaps surfaced by an end-to-end walkthrough of the 7-page onboarding flow (docs/index.mddocs/configuration/bootstrap-config.md) against main at 1a744ee. Three of the fixes (#1, #2, #3) close hard blockers on a clean machine (missing cargo / npm / provider-API-key prerequisites; model_not_found from display_name/model_name ambiguity; an Observability Exporter section whose second sentence is style-guide advice rather than user content). The remaining three (#4, #5, #6) close friction points along the same path (bootstrap-config.md field tables missing a Default column and an observability subsection; two credential-handling operational callouts; a cold-build duration note).

Doc-only diff. No code, schemas, configs, or test fixtures touched.

Closes#325.

Changes

FileChange
docs/quickstart/self-hosted.mdPrerequisites: add Rust 1.93+ with cargo (via rustup) as the leading prerequisite. After cargo run …: add a one-sentence note that the first-time build typically takes 3–5 minutes and that subsequent runs are incremental.
docs/quickstart/first-model-first-key-first-request.mdPrerequisites: replace "A reachable upstream OpenAI-compatible endpoint" with explicit credential-acquisition guidance (supported providers, sign-up link with cost order-of-magnitude, sk-... format hint, placeholder pointer). After the ProviderKey create-time response example: add a :::warning callout that the secret field is plaintext and that GET /admin/v1/provider_keys/:id also returns the plaintext.
docs/quickstart/openai-sdk.mdNew ## Prerequisites section placed between the "If you have not done that yet…" intro and the first ## What Changes In The SDK section. Lists a running gateway with provider key / model / API key already created, and Node.js 18+ with npm (the OpenAI SDK floor).
docs/overview/core-concepts.mdModel field-list bullet: inline-annotate display_name as the alias callers send in the API request's model field and model_name as the upstream model ID forwarded to the provider. API Key section: append a sentence after the key_hash description noting the plaintext is unrecoverable post-create and pointing at POST /admin/v1/apikeys/:id/rotate. Observability Exporter section: replace the documentation-author-facing second sentence with a user-facing description of what the resource does (OTLP-compatible export to external backends) and when to configure one.
docs/configuration/bootstrap-config.mdAdd a Default column to the field tables on etcd, proxy, admin. Build out the observability section with a new field table covering service_name, log_level, access_log, metrics.prometheus.*, metrics.otlp.*, and tracing.otlp.*, plus a pointer to the dynamic Observability Exporters page. Build out the cache section with a small field table covering backend and redis.

Net diff: 5 files, +58 / -35. Source: git diff --stat.

Test plan

Doc-only diff — no .rs files, schemas, configs, or test fixtures touched. The cargo trio was still run end-to-end as the canonical pre-merge gate.

  • cargo fmt --check — PASS (exit 0).
  • cargo clippy --workspace --all-targets -- -D warnings — PASS (exit 0; full tail captured below).
  • cargo test --workspace — PASS (exit 0; full crate-suite results captured below).
  • grep -rln <each affected page> tests/e2e/ for all 5 pages returns empty. pnpm test under tests/e2e/ is not applicable for this diff.

cargo clippy tail

 Checking aisix-proxy v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-proxy)
Checking aisix-admin v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-admin)
Checking aisix-server v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-server)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 40.28s

cargo test summary

33 test-suite-result lines, all 'ok'. Aggregate: 1029 tests passed, 0 failed, 3 ignored across the workspace (unit suites + doc-tests). Sample tail (last 3 named tests + last result line):
test keyword::tests::invalid_regex_is_a_clean_error_not_a_panic ... ok
test usage::tests::full_channel_drop_does_not_panic ... ok
test dispatch::tests::build_v1_url_rejects_path_without_leading_slash - should panic ... ok
test result: ok. <N> passed; 0 failed; <N> ignored; 0 measured; 0 filtered out

Affected pages

  • docs/quickstart/self-hosted.md
  • docs/quickstart/first-model-first-key-first-request.md
  • docs/quickstart/openai-sdk.md
  • docs/overview/core-concepts.md
  • docs/configuration/bootstrap-config.md

Pre-merge-check verification log

The three pre-merge checks called out in #325 were resolved as follows:

  1. Rust / Node minimum versions. Verified Rust against rust-toolchain.toml (channel = "1.93.1") and Cargo.toml (rust-version = "1.93"). The issue body's tentative "Rust 1.79+" floor was bumped to Rust 1.93 or newer in the doc, and a note added that rustup will pull the right channel automatically because rust-toolchain.toml pins it. For Node.js, neither tests/e2e/package.json nor a top-level package.json declares an engines floor, so the doc cites Node.js 18 or newer (the OpenAI SDK's published floor at version 4.x shipped in tests/e2e/package.json).
  2. Bootstrap-config Default column. Pulled all default values from crates/aisix-core/src/config.rs (this repo's config crate lives in aisix-core, not in a separate aisix-config crate as the issue body tentatively listed). Values pulled: EtcdConfig::default_prefix → "/aisix", default_dial_timeout / default_request_timeout → 5000ms, ProxyConfig::default_body_limit → 10 * 1024 * 1024, AdminConfig::default → addr "127.0.0.1:0" / admin_keys [] / tls None, ObservabilityConfig::default_service_name → "aisix", default_log_level → "info", default_access_log → true, PrometheusConfig::default → { enabled: true, path: "/metrics" }, OtlpConfig::default → { enabled: false, endpoint: None }, OtlpTracingConfig::default → { enabled: false, endpoint: None, sample_ratio: 1.0 }, CacheConfig::default → { backend: Memory, redis: None }. For fields without an explicit default and with Config::validate enforcement (etcd.endpoints, proxy.addr), the column reads required. For fields whose Default is None, the column reads none.
  3. Provider-key GET response shape.crates/aisix-core/src/models/provider_key.rs:39 defines pub secret: String without #[serde(skip_serializing)], and crates/aisix-admin/src/provider_keys_handlers.rs:29-38 (get_provider_key) returns Json<ResourceEntry<ProviderKey>> — so GET /admin/v1/provider_keys/:iddoes include the plaintext secret in the response body, every time. The original draft callout in docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages #325 read "subsequent reads via GET do not include it", which was empirically incorrect. The callout in this PR was adjusted to reflect actual behavior: the warning now states that the plaintext is in both the create response and any subsequent GET, and that the same handling applies any time you read the resource.

References

Summary by CodeRabbit

  • Documentation
    • Converted narrative configuration sections into structured tables for bootstrap, observability, and cache settings (including defaults and required/optional status)
    • Clarified core concepts: model display vs upstream identifiers, API key lifecycle/rotation, and observability exporter behavior
    • Added quickstart prerequisites: upstream provider API key, Node.js 18+ for OpenAI SDK, Rust 1.93+ for self-hosted
    • Added warning to treat provider API secrets as sensitive

Review Change Stack

CopilotAI review requested due to automatic review settings May 18, 2026 04:04
@janiussyafiqjaniussyafiq added documentation Improvements or additions to documentation priority-normal labels May 18, 2026
@coderabbitai

coderabbitaiBot commented May 18, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 2f8aa0cb-5faa-4a13-a703-ebe2423802be

📥 Commits

Reviewing files that changed from the base of the PR and between 5401ff1 and 1094aac.

📒 Files selected for processing (1)
  • docs/configuration/bootstrap-config.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/configuration/bootstrap-config.md

📝 Walkthrough

Walkthrough

This PR updates documentation: restructures bootstrap configuration sections into tables with defaults and optional flags, refines core concept fields and observability exporter text, and adds explicit quickstart prerequisites plus an API-key secrecy warning.

Changes

Documentation Clarity Improvements

Layer / File(s)Summary
Bootstrap configuration tables
docs/configuration/bootstrap-config.md
etcd, proxy, admin, observability, and cache "Important fields" are converted from narrative lists into tables that show default values, optional fields (e.g., tls), reserved telemetry keys, and conditional nested blocks (e.g., redis when backend: redis).
Core concept refinements
docs/overview/core-concepts.md
Model section now documents display_name, provider, and model_name; API Key section notes plaintext bearer tokens cannot be retrieved after creation and must be rotated; Observability Exporter text focuses on OTLP-compatible telemetry export.
Quickstart prerequisites and security guidance
docs/quickstart/openai-sdk.md, docs/quickstart/self-hosted.md, docs/quickstart/first-model-first-key-first-request.md
OpenAI SDK quickstart adds a Prerequisites section (gateway + Node.js 18+); self-hosted quickstart requires Rust 1.93+ and notes first cargo run compile time; first-model guide adds upstream provider API key prerequisite and warns that admin API returns secret in plaintext and it must be treated as sensitive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This doc-only PR improves new-developer onboarding by clarifying prerequisites, model naming, credential handling, and bootstrap configuration defaults across the quickstart and overview/configuration docs.

Changes:

  • Adds Rust/Node/provider-key prerequisite guidance and build-time expectations.
  • Clarifies model alias vs upstream model ID and credential visibility.
  • Expands bootstrap configuration tables, especially observability and cache fields.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
FileDescription
docs/quickstart/self-hosted.mdAdds Rust/cargo prerequisite and first-build duration note.
docs/quickstart/openai-sdk.mdAdds gateway and Node.js/npm prerequisites.
docs/quickstart/first-model-first-key-first-request.mdClarifies provider API key prerequisite and plaintext provider-key handling.
docs/overview/core-concepts.mdClarifies model fields, API-key plaintext handling, and observability exporter description.
docs/configuration/bootstrap-config.mdAdds defaults and expanded observability/cache bootstrap field tables.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threaddocs/overview/core-concepts.md Outdated

Current data-plane behavior is based on `key_hash`, not plaintext storage. The proxy hashes the incoming bearer token and resolves it against the stored `key_hash`.

This means you cannot retrieve the plaintext bearer after creation — capture the value returned at create time, and if you lose it, use `POST /admin/v1/apikeys/:id/rotate` to issue a new one.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Rewrote the appended paragraph to reflect actual behavior: the plaintext bearer is chosen (or generated) by the caller and SHA-256-hashed locally before submission, the gateway never sees or returns the plaintext at create time, and POST /admin/v1/apikeys/:id/rotate is the only endpoint that emits a server-generated plaintext (verified against apikeys_handlers.rs:97-104 for create and :155-178 for rotate). Pushed in commit 5401ff1.

Comment on lines +161 to +164
| `metrics.otlp.enabled` | push-style OTLP metrics exporter | `false` |
| `metrics.otlp.endpoint` | OTLP/gRPC collector endpoint, e.g. `"http://otel-collector:4317"` | none |
| `tracing.otlp.enabled` | push-style OTLP traces exporter | `false` |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces | none |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Confirmed against code: install_otlp_tracer (crates/aisix-obs/src/otlp.rs:26-58) validates the endpoint and emits a startup log line but defers the real exporter pipeline ("The real exporter wires up in a follow-up PR" per the inline code comment); no install_otlp_metrics function exists in the workspace. The bootstrap-config observability table now carries a Status column marking metrics.otlp.* as reserved (not yet wired) and tracing.otlp.* as partial (validation only). Pushed in commit 5401ff1.

Comment threaddocs/overview/core-concepts.md Outdated
An `Observability Exporter` is a resource that configures external telemetry export from the gateway.

Use this concept when documenting external metrics, traces, or event forwarding behavior.
An `Observability Exporter` ships telemetry (metrics, traces, request logs) from the gateway to an external backend over an OTLP-compatible endpoint (Grafana Tempo / Loki, Honeycomb, Langfuse via OTLP, and so on). Configure one when you want gateway request and response data forwarded to your existing observability stack.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against otlp_http_sink.rs:184/:249 (the exporter builds OTLP traces payloads from UsageEvent records — per-request spans, not general metrics or logs). Rewrote the section to: "ships per-request span telemetry — derived from gateway UsageEvent records — to an OTLP/HTTP-compatible backend". Pushed in commit 5401ff1.

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | scrape-style Prometheus exporter | `true` |
| `metrics.prometheus.path` | HTTP path the Prometheus exporter is served on | `"/metrics"` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-admin/src/lib.rs:68 (/metrics route is hardcoded with no conditional gating). The bootstrap-config observability table now marks metrics.prometheus.enabled and metrics.prometheus.path as reserved (not yet consulted), with a footnote that the Prometheus exporter is currently mounted unconditionally at the hardcoded admin /metrics path. Pushed in commit 5401ff1.

| --- | --- | --- |
| `service_name` | service-name attribute attached to every metric, log, and span emitted by the process | `"aisix"` |
| `log_level` | minimum log level (`error` / `warn` / `info` / `debug` / `trace`) | `"info"` |
| `access_log` | emit a structured access-log line for every proxy request | `true` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-obs/src/access_log.rs:32-50 (AccessLog::emit() always calls tracing::info! with no config gate) and the unconditional emit_access_log call sites in every proxy handler. The bootstrap-config observability table now marks access_log as reserved (not yet consulted), with a footnote that access logs are emitted unconditionally by every proxy handler today. Pushed in commit 5401ff1.

…lity-config claims to match runtime (Copilot review)
…s fields
PR #331 (commit e6125b6) wired the `metrics.prometheus.enabled` and
`metrics.prometheus.path` consumers in `crates/aisix-admin/src/lib.rs:154-159`
plus the `normalized_prometheus_path` helper at `:164-174`, and exercised
both paths end-to-end in three new unit tests
(`metrics_endpoint_uses_configured_path`,
`metrics_endpoint_normalizes_configured_path`,
`metrics_endpoint_can_be_disabled`).
The two rows in this PR's observability field table that called those
fields "reserved (not yet consulted)" are now factually stale. Update
both rows to `wired` and align the row descriptions with #331's
runtime behaviour. Also adjust the introductory paragraph above the
table so it no longer asserts that only `service_name` and `log_level`
are consulted at runtime.
This is the consumer-trace discipline locked in during round 2 of this
PR, applied prospectively to a moving target: PR #331 changed the
consumer site after round 2's verification window, so the doc text
needs to update.
CopilotAI review requested due to automatic review settings May 18, 2026 15:24

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

docs/configuration/bootstrap-config.md:164

  • The tracing endpoint is described as validated at boot, but the implementation only checks that tracing.otlp.enabled=true has a non-empty endpoint; it does not parse or validate the endpoint URL/protocol before returning a handle. Calling this "validated" overstates the current behavior.
| `tracing.otlp.enabled` | enabling this validates the endpoint at boot and emits a startup log line; the OTLP traces pipeline itself is deferred to a future release | `false` | partial (validation only) |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces; validated at boot when `tracing.otlp.enabled` is `true` | none | partial (validation only) |

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | controls whether the admin listener mounts the Prometheus scrape endpoint; when `false`, no `/metrics` route is registered | `true` | wired |
| `metrics.prometheus.path` | mount path for the Prometheus scrape endpoint when `metrics.prometheus.enabled` is `true`; values without a leading slash are normalised by prepending one, and an empty value falls back to `/metrics` | `"/metrics"` | wired |
## `observability`

Use `observability` to configure:
Use `observability` to set process-wide telemetry knobs: service name, log level, Prometheus exporter control, and (in future releases) access-log gating and OTLP exporters. Today `service_name`, `log_level`, and the `metrics.prometheus.*` block are consulted at runtime; the remaining keys are recognized in the schema and reserved for upcoming releases — setting them is harmless but currently has no effect.
@moonming
moonming merged commit e12155b into mainMay 19, 2026
11 checks passed
janiussyafiq added a commit that referenced this pull request May 20, 2026
Two related items folded into a single revision commit per operator
authorization (msg `a5c9dec7`):
1. Merge conflict resolution against `main` post-PR-#326
PR #326 (merge commit `e12155b`, landed 2026-05-19 02:30:04Z) updated
the `## Prerequisites` block and the `## Step 3: Start the gateway`
section of `docs/quickstart/self-hosted.md` on `main`. This branch's
prior edits (etcd glossary link, `config.yaml` location anchor,
`YOUR_ADMIN_KEY` placeholder format note, "Keep the gateway running"
framing) overlapped the Prerequisites bullet on the same physical
line.
Resolution per Umar's approved Option 2: single new commit on top of
`a109d2d` that re-applies this branch's Bucket-B edits on top of the
post-#326 canonical lines:
- Add the post-#326 Rust 1.93 prerequisite bullet.
- Capitalize "A reachable etcd instance" (per #326) while preserving
this branch's `[etcd](../overview/glossary.md#etcd)` glossary link.
- Add the post-#326 "first time you run this" cargo-build duration
paragraph between the `cargo run` code block and this branch's
"Keep the gateway running. Open a new terminal..." framing. The
two paragraphs now sequence as: (a) explain first-run build time,
(b) instruct reader to keep the gateway running and switch
terminals.
Other 4 overlapping files (`bootstrap-config.md`, `core-concepts.md`,
`first-model-first-key-first-request.md`, `openai-sdk.md`) auto-merge
cleanly against post-#326 main and are not touched by this commit.
2. Copilot review address — comment id `3260189475`
The reviewer flagged that the "Capture the id (jq path)" snippet on
`docs/quickstart/first-model-first-key-first-request.md` re-ran the
same `POST /admin/v1/provider_keys` as Step 1, which would either
trigger a duplicate-`display_name` rejection from the admin API or
leave the reader with two ProviderKey rows whose ids differ from the
response example shown after the first POST. Verified empirically by
reading the two POST blocks (lines 37-46 and the captured-id snippet)
against each other.
Restructure the section so the jq path is presented as a variant of
the original create, not a second POST:
- Rename the code block title to "Create and capture the id in one
shot" to make the variant relationship explicit.
- Wrap the two paths as bulleted Option A / Option B blocks. Option
A is the jq variant of the create above ("instead of the curl
shown earlier"). Option B is the jq-free path ("if you already
ran the curl above, copy the id field by eye").
- Prefix the section with an explicit "pick one, don't run both"
callout naming the duplicate-display_name rejection as the failure
mode if the reader runs both forms.
janiussyafiq added a commit that referenced this pull request May 20, 2026
Integrate origin/main (commit 2c1d485 = post-PR-#326 / #348 plus
#330 / #341 / #343 / #345 / #346) into this branch via `git merge
--squash` to clear PR #344's lingering `mergeable: dirty` state.
Conflict on `docs/quickstart/self-hosted.md` was a 3-way-merge-base
artifact: base (3596c0a) read `- a reachable etcd instance`, main
changed `a` → `A` (via #326), this branch additionally inserted the
glossary link. Both changes are wanted; resolution per Umar's
approved plan was `git checkout --ours`, which preserves the branch's
self-hosted.md state (already integrates capital A + glossary link
+ first-time-build paragraph + keep-running framing). Other 4
overlapping doc files auto-merged cleanly (`bootstrap-config.md`,
`core-concepts.md`, `first-model-first-key-first-request.md`,
`openai-sdk.md`). Code files all auto-merged cleanly.
Additional Copilot review (post-`167196a` cycle) addressed:
- `docs/index.md:7` — change link display text from `[data-plane]`
to `[data plane]` to match the canonical glossary term. The URL
anchor `#data-plane` stays kebab-case (matches the glossary
heading's auto-anchor); only the display text changes. Comment
id 3271145422.
- `docs/quickstart/openai-sdk.md:43` — change `All three steps below`
to `All commands below`. The Install-the-SDK section has two
command blocks (mkdir+cd, npm install), not three; the prior
wording originated from a mental model (mkdir, cd, install)
that doesn't match the typographic count of code blocks under
the heading. Comment id 3271145458.
Copilot's third comment on `docs/overview/core-concepts.md`
Observability Exporter wording (id 3271145444) auto-resolves via
this merge — main's #326 rewrite supersedes the branch's pre-#326
wording at that location ("ships per-request span telemetry…
OTLP/HTTP-compatible backend…" replaces "Use this concept when
documenting…"). No separate edit needed; the merge IS the fix.
janiussyafiq added a commit that referenced this pull request May 20, 2026
…ickstart-polish
Resolve PR #344's lingering mergeable: dirty state by linking the
branch history to origin/main (2c1d485 = post-#326 / #348 / #330 /
#341 / #343 / #345 / #346).
The squash-merge commit landed earlier (e2af197) integrated main's
content into the branch tree but did not link the histories, so
GitHub's mergeable computation still saw the 3-way-merge-base
artifact conflict on docs/quickstart/self-hosted.md (a vs A + the
glossary link / "In another terminal" vs "Keep the gateway running"
framing). This explicit merge commit ties the branch to main's
history.
Self-hosted.md conflict resolved by taking OUR side — the branch's
edits already contain main's substantive changes (capital A,
first-time-build paragraph) plus this PR's additions (glossary
link, keep-running framing, YOUR_ADMIN_KEY note, config.yaml
location anchor).
The auto-merge of first-model-first-key-first-request.md duplicated
the :::warning callout that was already integrated via the squash
commit; removed the duplicate.
moonming pushed a commit that referenced this pull request May 22, 2026
@jarvis9443
jarvis9443 deleted the docs/issue-325-onboarding-flow-fixes branch June 25, 2026 06:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationpriority-normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages

3 participants

@janiussyafiq@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages - #326

Merged
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes
May 19, 2026
Merged

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages#326
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes

Conversation

@janiussyafiq

@janiussyafiqjaniussyafiq commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes six new-developer onboarding gaps surfaced by an end-to-end walkthrough of the 7-page onboarding flow (docs/index.mddocs/configuration/bootstrap-config.md) against main at 1a744ee. Three of the fixes (#1, #2, #3) close hard blockers on a clean machine (missing cargo / npm / provider-API-key prerequisites; model_not_found from display_name/model_name ambiguity; an Observability Exporter section whose second sentence is style-guide advice rather than user content). The remaining three (#4, #5, #6) close friction points along the same path (bootstrap-config.md field tables missing a Default column and an observability subsection; two credential-handling operational callouts; a cold-build duration note).

Doc-only diff. No code, schemas, configs, or test fixtures touched.

Closes#325.

Changes

FileChange
docs/quickstart/self-hosted.mdPrerequisites: add Rust 1.93+ with cargo (via rustup) as the leading prerequisite. After cargo run …: add a one-sentence note that the first-time build typically takes 3–5 minutes and that subsequent runs are incremental.
docs/quickstart/first-model-first-key-first-request.mdPrerequisites: replace "A reachable upstream OpenAI-compatible endpoint" with explicit credential-acquisition guidance (supported providers, sign-up link with cost order-of-magnitude, sk-... format hint, placeholder pointer). After the ProviderKey create-time response example: add a :::warning callout that the secret field is plaintext and that GET /admin/v1/provider_keys/:id also returns the plaintext.
docs/quickstart/openai-sdk.mdNew ## Prerequisites section placed between the "If you have not done that yet…" intro and the first ## What Changes In The SDK section. Lists a running gateway with provider key / model / API key already created, and Node.js 18+ with npm (the OpenAI SDK floor).
docs/overview/core-concepts.mdModel field-list bullet: inline-annotate display_name as the alias callers send in the API request's model field and model_name as the upstream model ID forwarded to the provider. API Key section: append a sentence after the key_hash description noting the plaintext is unrecoverable post-create and pointing at POST /admin/v1/apikeys/:id/rotate. Observability Exporter section: replace the documentation-author-facing second sentence with a user-facing description of what the resource does (OTLP-compatible export to external backends) and when to configure one.
docs/configuration/bootstrap-config.mdAdd a Default column to the field tables on etcd, proxy, admin. Build out the observability section with a new field table covering service_name, log_level, access_log, metrics.prometheus.*, metrics.otlp.*, and tracing.otlp.*, plus a pointer to the dynamic Observability Exporters page. Build out the cache section with a small field table covering backend and redis.

Net diff: 5 files, +58 / -35. Source: git diff --stat.

Test plan

Doc-only diff — no .rs files, schemas, configs, or test fixtures touched. The cargo trio was still run end-to-end as the canonical pre-merge gate.

  • cargo fmt --check — PASS (exit 0).
  • cargo clippy --workspace --all-targets -- -D warnings — PASS (exit 0; full tail captured below).
  • cargo test --workspace — PASS (exit 0; full crate-suite results captured below).
  • grep -rln <each affected page> tests/e2e/ for all 5 pages returns empty. pnpm test under tests/e2e/ is not applicable for this diff.

cargo clippy tail

 Checking aisix-proxy v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-proxy)
Checking aisix-admin v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-admin)
Checking aisix-server v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-server)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 40.28s

cargo test summary

33 test-suite-result lines, all 'ok'. Aggregate: 1029 tests passed, 0 failed, 3 ignored across the workspace (unit suites + doc-tests). Sample tail (last 3 named tests + last result line):
test keyword::tests::invalid_regex_is_a_clean_error_not_a_panic ... ok
test usage::tests::full_channel_drop_does_not_panic ... ok
test dispatch::tests::build_v1_url_rejects_path_without_leading_slash - should panic ... ok
test result: ok. <N> passed; 0 failed; <N> ignored; 0 measured; 0 filtered out

Affected pages

  • docs/quickstart/self-hosted.md
  • docs/quickstart/first-model-first-key-first-request.md
  • docs/quickstart/openai-sdk.md
  • docs/overview/core-concepts.md
  • docs/configuration/bootstrap-config.md

Pre-merge-check verification log

The three pre-merge checks called out in #325 were resolved as follows:

  1. Rust / Node minimum versions. Verified Rust against rust-toolchain.toml (channel = "1.93.1") and Cargo.toml (rust-version = "1.93"). The issue body's tentative "Rust 1.79+" floor was bumped to Rust 1.93 or newer in the doc, and a note added that rustup will pull the right channel automatically because rust-toolchain.toml pins it. For Node.js, neither tests/e2e/package.json nor a top-level package.json declares an engines floor, so the doc cites Node.js 18 or newer (the OpenAI SDK's published floor at version 4.x shipped in tests/e2e/package.json).
  2. Bootstrap-config Default column. Pulled all default values from crates/aisix-core/src/config.rs (this repo's config crate lives in aisix-core, not in a separate aisix-config crate as the issue body tentatively listed). Values pulled: EtcdConfig::default_prefix → "/aisix", default_dial_timeout / default_request_timeout → 5000ms, ProxyConfig::default_body_limit → 10 * 1024 * 1024, AdminConfig::default → addr "127.0.0.1:0" / admin_keys [] / tls None, ObservabilityConfig::default_service_name → "aisix", default_log_level → "info", default_access_log → true, PrometheusConfig::default → { enabled: true, path: "/metrics" }, OtlpConfig::default → { enabled: false, endpoint: None }, OtlpTracingConfig::default → { enabled: false, endpoint: None, sample_ratio: 1.0 }, CacheConfig::default → { backend: Memory, redis: None }. For fields without an explicit default and with Config::validate enforcement (etcd.endpoints, proxy.addr), the column reads required. For fields whose Default is None, the column reads none.
  3. Provider-key GET response shape.crates/aisix-core/src/models/provider_key.rs:39 defines pub secret: String without #[serde(skip_serializing)], and crates/aisix-admin/src/provider_keys_handlers.rs:29-38 (get_provider_key) returns Json<ResourceEntry<ProviderKey>> — so GET /admin/v1/provider_keys/:iddoes include the plaintext secret in the response body, every time. The original draft callout in docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages #325 read "subsequent reads via GET do not include it", which was empirically incorrect. The callout in this PR was adjusted to reflect actual behavior: the warning now states that the plaintext is in both the create response and any subsequent GET, and that the same handling applies any time you read the resource.

References

Summary by CodeRabbit

  • Documentation
    • Converted narrative configuration sections into structured tables for bootstrap, observability, and cache settings (including defaults and required/optional status)
    • Clarified core concepts: model display vs upstream identifiers, API key lifecycle/rotation, and observability exporter behavior
    • Added quickstart prerequisites: upstream provider API key, Node.js 18+ for OpenAI SDK, Rust 1.93+ for self-hosted
    • Added warning to treat provider API secrets as sensitive

Review Change Stack

CopilotAI review requested due to automatic review settings May 18, 2026 04:04
@janiussyafiqjaniussyafiq added documentation Improvements or additions to documentation priority-normal labels May 18, 2026
@coderabbitai

coderabbitaiBot commented May 18, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 2f8aa0cb-5faa-4a13-a703-ebe2423802be

📥 Commits

Reviewing files that changed from the base of the PR and between 5401ff1 and 1094aac.

📒 Files selected for processing (1)
  • docs/configuration/bootstrap-config.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/configuration/bootstrap-config.md

📝 Walkthrough

Walkthrough

This PR updates documentation: restructures bootstrap configuration sections into tables with defaults and optional flags, refines core concept fields and observability exporter text, and adds explicit quickstart prerequisites plus an API-key secrecy warning.

Changes

Documentation Clarity Improvements

Layer / File(s)Summary
Bootstrap configuration tables
docs/configuration/bootstrap-config.md
etcd, proxy, admin, observability, and cache "Important fields" are converted from narrative lists into tables that show default values, optional fields (e.g., tls), reserved telemetry keys, and conditional nested blocks (e.g., redis when backend: redis).
Core concept refinements
docs/overview/core-concepts.md
Model section now documents display_name, provider, and model_name; API Key section notes plaintext bearer tokens cannot be retrieved after creation and must be rotated; Observability Exporter text focuses on OTLP-compatible telemetry export.
Quickstart prerequisites and security guidance
docs/quickstart/openai-sdk.md, docs/quickstart/self-hosted.md, docs/quickstart/first-model-first-key-first-request.md
OpenAI SDK quickstart adds a Prerequisites section (gateway + Node.js 18+); self-hosted quickstart requires Rust 1.93+ and notes first cargo run compile time; first-model guide adds upstream provider API key prerequisite and warns that admin API returns secret in plaintext and it must be treated as sensitive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This doc-only PR improves new-developer onboarding by clarifying prerequisites, model naming, credential handling, and bootstrap configuration defaults across the quickstart and overview/configuration docs.

Changes:

  • Adds Rust/Node/provider-key prerequisite guidance and build-time expectations.
  • Clarifies model alias vs upstream model ID and credential visibility.
  • Expands bootstrap configuration tables, especially observability and cache fields.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
FileDescription
docs/quickstart/self-hosted.mdAdds Rust/cargo prerequisite and first-build duration note.
docs/quickstart/openai-sdk.mdAdds gateway and Node.js/npm prerequisites.
docs/quickstart/first-model-first-key-first-request.mdClarifies provider API key prerequisite and plaintext provider-key handling.
docs/overview/core-concepts.mdClarifies model fields, API-key plaintext handling, and observability exporter description.
docs/configuration/bootstrap-config.mdAdds defaults and expanded observability/cache bootstrap field tables.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threaddocs/overview/core-concepts.md Outdated

Current data-plane behavior is based on `key_hash`, not plaintext storage. The proxy hashes the incoming bearer token and resolves it against the stored `key_hash`.

This means you cannot retrieve the plaintext bearer after creation — capture the value returned at create time, and if you lose it, use `POST /admin/v1/apikeys/:id/rotate` to issue a new one.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Rewrote the appended paragraph to reflect actual behavior: the plaintext bearer is chosen (or generated) by the caller and SHA-256-hashed locally before submission, the gateway never sees or returns the plaintext at create time, and POST /admin/v1/apikeys/:id/rotate is the only endpoint that emits a server-generated plaintext (verified against apikeys_handlers.rs:97-104 for create and :155-178 for rotate). Pushed in commit 5401ff1.

Comment on lines +161 to +164
| `metrics.otlp.enabled` | push-style OTLP metrics exporter | `false` |
| `metrics.otlp.endpoint` | OTLP/gRPC collector endpoint, e.g. `"http://otel-collector:4317"` | none |
| `tracing.otlp.enabled` | push-style OTLP traces exporter | `false` |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces | none |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Confirmed against code: install_otlp_tracer (crates/aisix-obs/src/otlp.rs:26-58) validates the endpoint and emits a startup log line but defers the real exporter pipeline ("The real exporter wires up in a follow-up PR" per the inline code comment); no install_otlp_metrics function exists in the workspace. The bootstrap-config observability table now carries a Status column marking metrics.otlp.* as reserved (not yet wired) and tracing.otlp.* as partial (validation only). Pushed in commit 5401ff1.

Comment threaddocs/overview/core-concepts.md Outdated
An `Observability Exporter` is a resource that configures external telemetry export from the gateway.

Use this concept when documenting external metrics, traces, or event forwarding behavior.
An `Observability Exporter` ships telemetry (metrics, traces, request logs) from the gateway to an external backend over an OTLP-compatible endpoint (Grafana Tempo / Loki, Honeycomb, Langfuse via OTLP, and so on). Configure one when you want gateway request and response data forwarded to your existing observability stack.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against otlp_http_sink.rs:184/:249 (the exporter builds OTLP traces payloads from UsageEvent records — per-request spans, not general metrics or logs). Rewrote the section to: "ships per-request span telemetry — derived from gateway UsageEvent records — to an OTLP/HTTP-compatible backend". Pushed in commit 5401ff1.

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | scrape-style Prometheus exporter | `true` |
| `metrics.prometheus.path` | HTTP path the Prometheus exporter is served on | `"/metrics"` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-admin/src/lib.rs:68 (/metrics route is hardcoded with no conditional gating). The bootstrap-config observability table now marks metrics.prometheus.enabled and metrics.prometheus.path as reserved (not yet consulted), with a footnote that the Prometheus exporter is currently mounted unconditionally at the hardcoded admin /metrics path. Pushed in commit 5401ff1.

| --- | --- | --- |
| `service_name` | service-name attribute attached to every metric, log, and span emitted by the process | `"aisix"` |
| `log_level` | minimum log level (`error` / `warn` / `info` / `debug` / `trace`) | `"info"` |
| `access_log` | emit a structured access-log line for every proxy request | `true` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-obs/src/access_log.rs:32-50 (AccessLog::emit() always calls tracing::info! with no config gate) and the unconditional emit_access_log call sites in every proxy handler. The bootstrap-config observability table now marks access_log as reserved (not yet consulted), with a footnote that access logs are emitted unconditionally by every proxy handler today. Pushed in commit 5401ff1.

…lity-config claims to match runtime (Copilot review)
…s fields
PR #331 (commit e6125b6) wired the `metrics.prometheus.enabled` and
`metrics.prometheus.path` consumers in `crates/aisix-admin/src/lib.rs:154-159`
plus the `normalized_prometheus_path` helper at `:164-174`, and exercised
both paths end-to-end in three new unit tests
(`metrics_endpoint_uses_configured_path`,
`metrics_endpoint_normalizes_configured_path`,
`metrics_endpoint_can_be_disabled`).
The two rows in this PR's observability field table that called those
fields "reserved (not yet consulted)" are now factually stale. Update
both rows to `wired` and align the row descriptions with #331's
runtime behaviour. Also adjust the introductory paragraph above the
table so it no longer asserts that only `service_name` and `log_level`
are consulted at runtime.
This is the consumer-trace discipline locked in during round 2 of this
PR, applied prospectively to a moving target: PR #331 changed the
consumer site after round 2's verification window, so the doc text
needs to update.
CopilotAI review requested due to automatic review settings May 18, 2026 15:24

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

docs/configuration/bootstrap-config.md:164

  • The tracing endpoint is described as validated at boot, but the implementation only checks that tracing.otlp.enabled=true has a non-empty endpoint; it does not parse or validate the endpoint URL/protocol before returning a handle. Calling this "validated" overstates the current behavior.
| `tracing.otlp.enabled` | enabling this validates the endpoint at boot and emits a startup log line; the OTLP traces pipeline itself is deferred to a future release | `false` | partial (validation only) |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces; validated at boot when `tracing.otlp.enabled` is `true` | none | partial (validation only) |

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | controls whether the admin listener mounts the Prometheus scrape endpoint; when `false`, no `/metrics` route is registered | `true` | wired |
| `metrics.prometheus.path` | mount path for the Prometheus scrape endpoint when `metrics.prometheus.enabled` is `true`; values without a leading slash are normalised by prepending one, and an empty value falls back to `/metrics` | `"/metrics"` | wired |
## `observability`

Use `observability` to configure:
Use `observability` to set process-wide telemetry knobs: service name, log level, Prometheus exporter control, and (in future releases) access-log gating and OTLP exporters. Today `service_name`, `log_level`, and the `metrics.prometheus.*` block are consulted at runtime; the remaining keys are recognized in the schema and reserved for upcoming releases — setting them is harmless but currently has no effect.
@moonming
moonming merged commit e12155b into mainMay 19, 2026
11 checks passed
janiussyafiq added a commit that referenced this pull request May 20, 2026
Two related items folded into a single revision commit per operator
authorization (msg `a5c9dec7`):
1. Merge conflict resolution against `main` post-PR-#326
PR #326 (merge commit `e12155b`, landed 2026-05-19 02:30:04Z) updated
the `## Prerequisites` block and the `## Step 3: Start the gateway`
section of `docs/quickstart/self-hosted.md` on `main`. This branch's
prior edits (etcd glossary link, `config.yaml` location anchor,
`YOUR_ADMIN_KEY` placeholder format note, "Keep the gateway running"
framing) overlapped the Prerequisites bullet on the same physical
line.
Resolution per Umar's approved Option 2: single new commit on top of
`a109d2d` that re-applies this branch's Bucket-B edits on top of the
post-#326 canonical lines:
- Add the post-#326 Rust 1.93 prerequisite bullet.
- Capitalize "A reachable etcd instance" (per #326) while preserving
this branch's `[etcd](../overview/glossary.md#etcd)` glossary link.
- Add the post-#326 "first time you run this" cargo-build duration
paragraph between the `cargo run` code block and this branch's
"Keep the gateway running. Open a new terminal..." framing. The
two paragraphs now sequence as: (a) explain first-run build time,
(b) instruct reader to keep the gateway running and switch
terminals.
Other 4 overlapping files (`bootstrap-config.md`, `core-concepts.md`,
`first-model-first-key-first-request.md`, `openai-sdk.md`) auto-merge
cleanly against post-#326 main and are not touched by this commit.
2. Copilot review address — comment id `3260189475`
The reviewer flagged that the "Capture the id (jq path)" snippet on
`docs/quickstart/first-model-first-key-first-request.md` re-ran the
same `POST /admin/v1/provider_keys` as Step 1, which would either
trigger a duplicate-`display_name` rejection from the admin API or
leave the reader with two ProviderKey rows whose ids differ from the
response example shown after the first POST. Verified empirically by
reading the two POST blocks (lines 37-46 and the captured-id snippet)
against each other.
Restructure the section so the jq path is presented as a variant of
the original create, not a second POST:
- Rename the code block title to "Create and capture the id in one
shot" to make the variant relationship explicit.
- Wrap the two paths as bulleted Option A / Option B blocks. Option
A is the jq variant of the create above ("instead of the curl
shown earlier"). Option B is the jq-free path ("if you already
ran the curl above, copy the id field by eye").
- Prefix the section with an explicit "pick one, don't run both"
callout naming the duplicate-display_name rejection as the failure
mode if the reader runs both forms.
janiussyafiq added a commit that referenced this pull request May 20, 2026
Integrate origin/main (commit 2c1d485 = post-PR-#326 / #348 plus
#330 / #341 / #343 / #345 / #346) into this branch via `git merge
--squash` to clear PR #344's lingering `mergeable: dirty` state.
Conflict on `docs/quickstart/self-hosted.md` was a 3-way-merge-base
artifact: base (3596c0a) read `- a reachable etcd instance`, main
changed `a` → `A` (via #326), this branch additionally inserted the
glossary link. Both changes are wanted; resolution per Umar's
approved plan was `git checkout --ours`, which preserves the branch's
self-hosted.md state (already integrates capital A + glossary link
+ first-time-build paragraph + keep-running framing). Other 4
overlapping doc files auto-merged cleanly (`bootstrap-config.md`,
`core-concepts.md`, `first-model-first-key-first-request.md`,
`openai-sdk.md`). Code files all auto-merged cleanly.
Additional Copilot review (post-`167196a` cycle) addressed:
- `docs/index.md:7` — change link display text from `[data-plane]`
to `[data plane]` to match the canonical glossary term. The URL
anchor `#data-plane` stays kebab-case (matches the glossary
heading's auto-anchor); only the display text changes. Comment
id 3271145422.
- `docs/quickstart/openai-sdk.md:43` — change `All three steps below`
to `All commands below`. The Install-the-SDK section has two
command blocks (mkdir+cd, npm install), not three; the prior
wording originated from a mental model (mkdir, cd, install)
that doesn't match the typographic count of code blocks under
the heading. Comment id 3271145458.
Copilot's third comment on `docs/overview/core-concepts.md`
Observability Exporter wording (id 3271145444) auto-resolves via
this merge — main's #326 rewrite supersedes the branch's pre-#326
wording at that location ("ships per-request span telemetry…
OTLP/HTTP-compatible backend…" replaces "Use this concept when
documenting…"). No separate edit needed; the merge IS the fix.
janiussyafiq added a commit that referenced this pull request May 20, 2026
…ickstart-polish
Resolve PR #344's lingering mergeable: dirty state by linking the
branch history to origin/main (2c1d485 = post-#326 / #348 / #330 /
#341 / #343 / #345 / #346).
The squash-merge commit landed earlier (e2af197) integrated main's
content into the branch tree but did not link the histories, so
GitHub's mergeable computation still saw the 3-way-merge-base
artifact conflict on docs/quickstart/self-hosted.md (a vs A + the
glossary link / "In another terminal" vs "Keep the gateway running"
framing). This explicit merge commit ties the branch to main's
history.
Self-hosted.md conflict resolved by taking OUR side — the branch's
edits already contain main's substantive changes (capital A,
first-time-build paragraph) plus this PR's additions (glossary
link, keep-running framing, YOUR_ADMIN_KEY note, config.yaml
location anchor).
The auto-merge of first-model-first-key-first-request.md duplicated
the :::warning callout that was already integrated via the squash
commit; removed the duplicate.
moonming pushed a commit that referenced this pull request May 22, 2026
@jarvis9443
jarvis9443 deleted the docs/issue-325-onboarding-flow-fixes branch June 25, 2026 06:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationpriority-normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages

3 participants

@janiussyafiq@moonming
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages - #326

Merged
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes
May 19, 2026
Merged

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages#326
moonming merged 3 commits into
mainfrom
docs/issue-325-onboarding-flow-fixes

Conversation

@janiussyafiq

@janiussyafiqjaniussyafiq commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes six new-developer onboarding gaps surfaced by an end-to-end walkthrough of the 7-page onboarding flow (docs/index.mddocs/configuration/bootstrap-config.md) against main at 1a744ee. Three of the fixes (#1, #2, #3) close hard blockers on a clean machine (missing cargo / npm / provider-API-key prerequisites; model_not_found from display_name/model_name ambiguity; an Observability Exporter section whose second sentence is style-guide advice rather than user content). The remaining three (#4, #5, #6) close friction points along the same path (bootstrap-config.md field tables missing a Default column and an observability subsection; two credential-handling operational callouts; a cold-build duration note).

Doc-only diff. No code, schemas, configs, or test fixtures touched.

Closes#325.

Changes

FileChange
docs/quickstart/self-hosted.mdPrerequisites: add Rust 1.93+ with cargo (via rustup) as the leading prerequisite. After cargo run …: add a one-sentence note that the first-time build typically takes 3–5 minutes and that subsequent runs are incremental.
docs/quickstart/first-model-first-key-first-request.mdPrerequisites: replace "A reachable upstream OpenAI-compatible endpoint" with explicit credential-acquisition guidance (supported providers, sign-up link with cost order-of-magnitude, sk-... format hint, placeholder pointer). After the ProviderKey create-time response example: add a :::warning callout that the secret field is plaintext and that GET /admin/v1/provider_keys/:id also returns the plaintext.
docs/quickstart/openai-sdk.mdNew ## Prerequisites section placed between the "If you have not done that yet…" intro and the first ## What Changes In The SDK section. Lists a running gateway with provider key / model / API key already created, and Node.js 18+ with npm (the OpenAI SDK floor).
docs/overview/core-concepts.mdModel field-list bullet: inline-annotate display_name as the alias callers send in the API request's model field and model_name as the upstream model ID forwarded to the provider. API Key section: append a sentence after the key_hash description noting the plaintext is unrecoverable post-create and pointing at POST /admin/v1/apikeys/:id/rotate. Observability Exporter section: replace the documentation-author-facing second sentence with a user-facing description of what the resource does (OTLP-compatible export to external backends) and when to configure one.
docs/configuration/bootstrap-config.mdAdd a Default column to the field tables on etcd, proxy, admin. Build out the observability section with a new field table covering service_name, log_level, access_log, metrics.prometheus.*, metrics.otlp.*, and tracing.otlp.*, plus a pointer to the dynamic Observability Exporters page. Build out the cache section with a small field table covering backend and redis.

Net diff: 5 files, +58 / -35. Source: git diff --stat.

Test plan

Doc-only diff — no .rs files, schemas, configs, or test fixtures touched. The cargo trio was still run end-to-end as the canonical pre-merge gate.

  • cargo fmt --check — PASS (exit 0).
  • cargo clippy --workspace --all-targets -- -D warnings — PASS (exit 0; full tail captured below).
  • cargo test --workspace — PASS (exit 0; full crate-suite results captured below).
  • grep -rln <each affected page> tests/e2e/ for all 5 pages returns empty. pnpm test under tests/e2e/ is not applicable for this diff.

cargo clippy tail

 Checking aisix-proxy v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-proxy)
Checking aisix-admin v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-admin)
Checking aisix-server v0.1.0 (/root/GitHub/ai-gateway/crates/aisix-server)
Finished `dev` profile [unoptimized + debuginfo] target(s) in 40.28s

cargo test summary

33 test-suite-result lines, all 'ok'. Aggregate: 1029 tests passed, 0 failed, 3 ignored across the workspace (unit suites + doc-tests). Sample tail (last 3 named tests + last result line):
test keyword::tests::invalid_regex_is_a_clean_error_not_a_panic ... ok
test usage::tests::full_channel_drop_does_not_panic ... ok
test dispatch::tests::build_v1_url_rejects_path_without_leading_slash - should panic ... ok
test result: ok. <N> passed; 0 failed; <N> ignored; 0 measured; 0 filtered out

Affected pages

  • docs/quickstart/self-hosted.md
  • docs/quickstart/first-model-first-key-first-request.md
  • docs/quickstart/openai-sdk.md
  • docs/overview/core-concepts.md
  • docs/configuration/bootstrap-config.md

Pre-merge-check verification log

The three pre-merge checks called out in #325 were resolved as follows:

  1. Rust / Node minimum versions. Verified Rust against rust-toolchain.toml (channel = "1.93.1") and Cargo.toml (rust-version = "1.93"). The issue body's tentative "Rust 1.79+" floor was bumped to Rust 1.93 or newer in the doc, and a note added that rustup will pull the right channel automatically because rust-toolchain.toml pins it. For Node.js, neither tests/e2e/package.json nor a top-level package.json declares an engines floor, so the doc cites Node.js 18 or newer (the OpenAI SDK's published floor at version 4.x shipped in tests/e2e/package.json).
  2. Bootstrap-config Default column. Pulled all default values from crates/aisix-core/src/config.rs (this repo's config crate lives in aisix-core, not in a separate aisix-config crate as the issue body tentatively listed). Values pulled: EtcdConfig::default_prefix → "/aisix", default_dial_timeout / default_request_timeout → 5000ms, ProxyConfig::default_body_limit → 10 * 1024 * 1024, AdminConfig::default → addr "127.0.0.1:0" / admin_keys [] / tls None, ObservabilityConfig::default_service_name → "aisix", default_log_level → "info", default_access_log → true, PrometheusConfig::default → { enabled: true, path: "/metrics" }, OtlpConfig::default → { enabled: false, endpoint: None }, OtlpTracingConfig::default → { enabled: false, endpoint: None, sample_ratio: 1.0 }, CacheConfig::default → { backend: Memory, redis: None }. For fields without an explicit default and with Config::validate enforcement (etcd.endpoints, proxy.addr), the column reads required. For fields whose Default is None, the column reads none.
  3. Provider-key GET response shape.crates/aisix-core/src/models/provider_key.rs:39 defines pub secret: String without #[serde(skip_serializing)], and crates/aisix-admin/src/provider_keys_handlers.rs:29-38 (get_provider_key) returns Json<ResourceEntry<ProviderKey>> — so GET /admin/v1/provider_keys/:iddoes include the plaintext secret in the response body, every time. The original draft callout in docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages #325 read "subsequent reads via GET do not include it", which was empirically incorrect. The callout in this PR was adjusted to reflect actual behavior: the warning now states that the plaintext is in both the create response and any subsequent GET, and that the same handling applies any time you read the resource.

References

Summary by CodeRabbit

  • Documentation
    • Converted narrative configuration sections into structured tables for bootstrap, observability, and cache settings (including defaults and required/optional status)
    • Clarified core concepts: model display vs upstream identifiers, API key lifecycle/rotation, and observability exporter behavior
    • Added quickstart prerequisites: upstream provider API key, Node.js 18+ for OpenAI SDK, Rust 1.93+ for self-hosted
    • Added warning to treat provider API secrets as sensitive

Review Change Stack

CopilotAI review requested due to automatic review settings May 18, 2026 04:04
@janiussyafiqjaniussyafiq added documentation Improvements or additions to documentation priority-normal labels May 18, 2026
@coderabbitai

coderabbitaiBot commented May 18, 2026

Copy link
Copy Markdown
ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Free

Run ID: 2f8aa0cb-5faa-4a13-a703-ebe2423802be

📥 Commits

Reviewing files that changed from the base of the PR and between 5401ff1 and 1094aac.

📒 Files selected for processing (1)
  • docs/configuration/bootstrap-config.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/configuration/bootstrap-config.md

📝 Walkthrough

Walkthrough

This PR updates documentation: restructures bootstrap configuration sections into tables with defaults and optional flags, refines core concept fields and observability exporter text, and adds explicit quickstart prerequisites plus an API-key secrecy warning.

Changes

Documentation Clarity Improvements

Layer / File(s)Summary
Bootstrap configuration tables
docs/configuration/bootstrap-config.md
etcd, proxy, admin, observability, and cache "Important fields" are converted from narrative lists into tables that show default values, optional fields (e.g., tls), reserved telemetry keys, and conditional nested blocks (e.g., redis when backend: redis).
Core concept refinements
docs/overview/core-concepts.md
Model section now documents display_name, provider, and model_name; API Key section notes plaintext bearer tokens cannot be retrieved after creation and must be rotated; Observability Exporter text focuses on OTLP-compatible telemetry export.
Quickstart prerequisites and security guidance
docs/quickstart/openai-sdk.md, docs/quickstart/self-hosted.md, docs/quickstart/first-model-first-key-first-request.md
OpenAI SDK quickstart adds a Prerequisites section (gateway + Node.js 18+); self-hosted quickstart requires Rust 1.93+ and notes first cargo run compile time; first-model guide adds upstream provider API key prerequisite and warns that admin API returns secret in plaintext and it must be treated as sensitive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes


Note

🎁 Summarized by CodeRabbit Free

Your organization has reached its limit of developer seats under the Pro Plan. For new users, CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please add seats to your subscription by visiting https://app.coderabbit.ai/login.If you believe this is a mistake and have available seats, please assign one to the pull request author through the subscription management page using the link above.

Comment @coderabbitai help to get the list of available commands and usage tips.

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This doc-only PR improves new-developer onboarding by clarifying prerequisites, model naming, credential handling, and bootstrap configuration defaults across the quickstart and overview/configuration docs.

Changes:

  • Adds Rust/Node/provider-key prerequisite guidance and build-time expectations.
  • Clarifies model alias vs upstream model ID and credential visibility.
  • Expands bootstrap configuration tables, especially observability and cache fields.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Show a summary per file
FileDescription
docs/quickstart/self-hosted.mdAdds Rust/cargo prerequisite and first-build duration note.
docs/quickstart/openai-sdk.mdAdds gateway and Node.js/npm prerequisites.
docs/quickstart/first-model-first-key-first-request.mdClarifies provider API key prerequisite and plaintext provider-key handling.
docs/overview/core-concepts.mdClarifies model fields, API-key plaintext handling, and observability exporter description.
docs/configuration/bootstrap-config.mdAdds defaults and expanded observability/cache bootstrap field tables.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threaddocs/overview/core-concepts.md Outdated

Current data-plane behavior is based on `key_hash`, not plaintext storage. The proxy hashes the incoming bearer token and resolves it against the stored `key_hash`.

This means you cannot retrieve the plaintext bearer after creation — capture the value returned at create time, and if you lose it, use `POST /admin/v1/apikeys/:id/rotate` to issue a new one.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Rewrote the appended paragraph to reflect actual behavior: the plaintext bearer is chosen (or generated) by the caller and SHA-256-hashed locally before submission, the gateway never sees or returns the plaintext at create time, and POST /admin/v1/apikeys/:id/rotate is the only endpoint that emits a server-generated plaintext (verified against apikeys_handlers.rs:97-104 for create and :155-178 for rotate). Pushed in commit 5401ff1.

Comment on lines +161 to +164
| `metrics.otlp.enabled` | push-style OTLP metrics exporter | `false` |
| `metrics.otlp.endpoint` | OTLP/gRPC collector endpoint, e.g. `"http://otel-collector:4317"` | none |
| `tracing.otlp.enabled` | push-style OTLP traces exporter | `false` |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces | none |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Confirmed against code: install_otlp_tracer (crates/aisix-obs/src/otlp.rs:26-58) validates the endpoint and emits a startup log line but defers the real exporter pipeline ("The real exporter wires up in a follow-up PR" per the inline code comment); no install_otlp_metrics function exists in the workspace. The bootstrap-config observability table now carries a Status column marking metrics.otlp.* as reserved (not yet wired) and tracing.otlp.* as partial (validation only). Pushed in commit 5401ff1.

Comment threaddocs/overview/core-concepts.md Outdated
An `Observability Exporter` is a resource that configures external telemetry export from the gateway.

Use this concept when documenting external metrics, traces, or event forwarding behavior.
An `Observability Exporter` ships telemetry (metrics, traces, request logs) from the gateway to an external backend over an OTLP-compatible endpoint (Grafana Tempo / Loki, Honeycomb, Langfuse via OTLP, and so on). Configure one when you want gateway request and response data forwarded to your existing observability stack.

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against otlp_http_sink.rs:184/:249 (the exporter builds OTLP traces payloads from UsageEvent records — per-request spans, not general metrics or logs). Rewrote the section to: "ships per-request span telemetry — derived from gateway UsageEvent records — to an OTLP/HTTP-compatible backend". Pushed in commit 5401ff1.

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | scrape-style Prometheus exporter | `true` |
| `metrics.prometheus.path` | HTTP path the Prometheus exporter is served on | `"/metrics"` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-admin/src/lib.rs:68 (/metrics route is hardcoded with no conditional gating). The bootstrap-config observability table now marks metrics.prometheus.enabled and metrics.prometheus.path as reserved (not yet consulted), with a footnote that the Prometheus exporter is currently mounted unconditionally at the hardcoded admin /metrics path. Pushed in commit 5401ff1.

| --- | --- | --- |
| `service_name` | service-name attribute attached to every metric, log, and span emitted by the process | `"aisix"` |
| `log_level` | minimum log level (`error` / `warn` / `info` / `debug` / `trace`) | `"info"` |
| `access_log` | emit a structured access-log line for every proxy request | `true` |

Copy link
Copy Markdown
CollaboratorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied. Verified against crates/aisix-obs/src/access_log.rs:32-50 (AccessLog::emit() always calls tracing::info! with no config gate) and the unconditional emit_access_log call sites in every proxy handler. The bootstrap-config observability table now marks access_log as reserved (not yet consulted), with a footnote that access logs are emitted unconditionally by every proxy handler today. Pushed in commit 5401ff1.

…lity-config claims to match runtime (Copilot review)
…s fields
PR #331 (commit e6125b6) wired the `metrics.prometheus.enabled` and
`metrics.prometheus.path` consumers in `crates/aisix-admin/src/lib.rs:154-159`
plus the `normalized_prometheus_path` helper at `:164-174`, and exercised
both paths end-to-end in three new unit tests
(`metrics_endpoint_uses_configured_path`,
`metrics_endpoint_normalizes_configured_path`,
`metrics_endpoint_can_be_disabled`).
The two rows in this PR's observability field table that called those
fields "reserved (not yet consulted)" are now factually stale. Update
both rows to `wired` and align the row descriptions with #331's
runtime behaviour. Also adjust the introductory paragraph above the
table so it no longer asserts that only `service_name` and `log_level`
are consulted at runtime.
This is the consumer-trace discipline locked in during round 2 of this
PR, applied prospectively to a moving target: PR #331 changed the
consumer site after round 2's verification window, so the doc text
needs to update.
CopilotAI review requested due to automatic review settings May 18, 2026 15:24

CopilotAI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

docs/configuration/bootstrap-config.md:164

  • The tracing endpoint is described as validated at boot, but the implementation only checks that tracing.otlp.enabled=true has a non-empty endpoint; it does not parse or validate the endpoint URL/protocol before returning a handle. Calling this "validated" overstates the current behavior.
| `tracing.otlp.enabled` | enabling this validates the endpoint at boot and emits a startup log line; the OTLP traces pipeline itself is deferred to a future release | `false` | partial (validation only) |
| `tracing.otlp.endpoint` | OTLP/gRPC collector endpoint for traces; validated at boot when `tracing.otlp.enabled` is `true` | none | partial (validation only) |

Comment on lines +159 to +160
| `metrics.prometheus.enabled` | controls whether the admin listener mounts the Prometheus scrape endpoint; when `false`, no `/metrics` route is registered | `true` | wired |
| `metrics.prometheus.path` | mount path for the Prometheus scrape endpoint when `metrics.prometheus.enabled` is `true`; values without a leading slash are normalised by prepending one, and an empty value falls back to `/metrics` | `"/metrics"` | wired |
## `observability`

Use `observability` to configure:
Use `observability` to set process-wide telemetry knobs: service name, log level, Prometheus exporter control, and (in future releases) access-log gating and OTLP exporters. Today `service_name`, `log_level`, and the `metrics.prometheus.*` block are consulted at runtime; the remaining keys are recognized in the schema and reserved for upcoming releases — setting them is harmless but currently has no effect.
@moonming
moonming merged commit e12155b into mainMay 19, 2026
11 checks passed
janiussyafiq added a commit that referenced this pull request May 20, 2026
Two related items folded into a single revision commit per operator
authorization (msg `a5c9dec7`):
1. Merge conflict resolution against `main` post-PR-#326
PR #326 (merge commit `e12155b`, landed 2026-05-19 02:30:04Z) updated
the `## Prerequisites` block and the `## Step 3: Start the gateway`
section of `docs/quickstart/self-hosted.md` on `main`. This branch's
prior edits (etcd glossary link, `config.yaml` location anchor,
`YOUR_ADMIN_KEY` placeholder format note, "Keep the gateway running"
framing) overlapped the Prerequisites bullet on the same physical
line.
Resolution per Umar's approved Option 2: single new commit on top of
`a109d2d` that re-applies this branch's Bucket-B edits on top of the
post-#326 canonical lines:
- Add the post-#326 Rust 1.93 prerequisite bullet.
- Capitalize "A reachable etcd instance" (per #326) while preserving
this branch's `[etcd](../overview/glossary.md#etcd)` glossary link.
- Add the post-#326 "first time you run this" cargo-build duration
paragraph between the `cargo run` code block and this branch's
"Keep the gateway running. Open a new terminal..." framing. The
two paragraphs now sequence as: (a) explain first-run build time,
(b) instruct reader to keep the gateway running and switch
terminals.
Other 4 overlapping files (`bootstrap-config.md`, `core-concepts.md`,
`first-model-first-key-first-request.md`, `openai-sdk.md`) auto-merge
cleanly against post-#326 main and are not touched by this commit.
2. Copilot review address — comment id `3260189475`
The reviewer flagged that the "Capture the id (jq path)" snippet on
`docs/quickstart/first-model-first-key-first-request.md` re-ran the
same `POST /admin/v1/provider_keys` as Step 1, which would either
trigger a duplicate-`display_name` rejection from the admin API or
leave the reader with two ProviderKey rows whose ids differ from the
response example shown after the first POST. Verified empirically by
reading the two POST blocks (lines 37-46 and the captured-id snippet)
against each other.
Restructure the section so the jq path is presented as a variant of
the original create, not a second POST:
- Rename the code block title to "Create and capture the id in one
shot" to make the variant relationship explicit.
- Wrap the two paths as bulleted Option A / Option B blocks. Option
A is the jq variant of the create above ("instead of the curl
shown earlier"). Option B is the jq-free path ("if you already
ran the curl above, copy the id field by eye").
- Prefix the section with an explicit "pick one, don't run both"
callout naming the duplicate-display_name rejection as the failure
mode if the reader runs both forms.
janiussyafiq added a commit that referenced this pull request May 20, 2026
Integrate origin/main (commit 2c1d485 = post-PR-#326 / #348 plus
#330 / #341 / #343 / #345 / #346) into this branch via `git merge
--squash` to clear PR #344's lingering `mergeable: dirty` state.
Conflict on `docs/quickstart/self-hosted.md` was a 3-way-merge-base
artifact: base (3596c0a) read `- a reachable etcd instance`, main
changed `a` → `A` (via #326), this branch additionally inserted the
glossary link. Both changes are wanted; resolution per Umar's
approved plan was `git checkout --ours`, which preserves the branch's
self-hosted.md state (already integrates capital A + glossary link
+ first-time-build paragraph + keep-running framing). Other 4
overlapping doc files auto-merged cleanly (`bootstrap-config.md`,
`core-concepts.md`, `first-model-first-key-first-request.md`,
`openai-sdk.md`). Code files all auto-merged cleanly.
Additional Copilot review (post-`167196a` cycle) addressed:
- `docs/index.md:7` — change link display text from `[data-plane]`
to `[data plane]` to match the canonical glossary term. The URL
anchor `#data-plane` stays kebab-case (matches the glossary
heading's auto-anchor); only the display text changes. Comment
id 3271145422.
- `docs/quickstart/openai-sdk.md:43` — change `All three steps below`
to `All commands below`. The Install-the-SDK section has two
command blocks (mkdir+cd, npm install), not three; the prior
wording originated from a mental model (mkdir, cd, install)
that doesn't match the typographic count of code blocks under
the heading. Comment id 3271145458.
Copilot's third comment on `docs/overview/core-concepts.md`
Observability Exporter wording (id 3271145444) auto-resolves via
this merge — main's #326 rewrite supersedes the branch's pre-#326
wording at that location ("ships per-request span telemetry…
OTLP/HTTP-compatible backend…" replaces "Use this concept when
documenting…"). No separate edit needed; the merge IS the fix.
janiussyafiq added a commit that referenced this pull request May 20, 2026
…ickstart-polish
Resolve PR #344's lingering mergeable: dirty state by linking the
branch history to origin/main (2c1d485 = post-#326 / #348 / #330 /
#341 / #343 / #345 / #346).
The squash-merge commit landed earlier (e2af197) integrated main's
content into the branch tree but did not link the histories, so
GitHub's mergeable computation still saw the 3-way-merge-base
artifact conflict on docs/quickstart/self-hosted.md (a vs A + the
glossary link / "In another terminal" vs "Keep the gateway running"
framing). This explicit merge commit ties the branch to main's
history.
Self-hosted.md conflict resolved by taking OUR side — the branch's
edits already contain main's substantive changes (capital A,
first-time-build paragraph) plus this PR's additions (glossary
link, keep-running framing, YOUR_ADMIN_KEY note, config.yaml
location anchor).
The auto-merge of first-model-first-key-first-request.md duplicated
the :::warning callout that was already integrated via the squash
commit; removed the duplicate.
moonming pushed a commit that referenced this pull request May 22, 2026
@jarvis9443
jarvis9443 deleted the docs/issue-325-onboarding-flow-fixes branch June 25, 2026 06:25
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationpriority-normal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

docs: fix new-developer onboarding gaps across overview, quickstart, and configuration pages

3 participants

@janiussyafiq@moonming