Add multi-registry documentation with schema v2 format#1190
Add multi-registry documentation with schema v2 format#1190
Conversation
Co-authored-by: jsuereth <29006+jsuereth@users.noreply.github.com>
… note Co-authored-by: jsuereth <29006+jsuereth@users.noreply.github.com>
Co-authored-by: jsuereth <29006+jsuereth@users.noreply.github.com>
Co-authored-by: jsuereth <29006+jsuereth@users.noreply.github.com>
Co-authored-by: jsuereth <29006+jsuereth@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1190 +/- ##
=====================================
Coverage 79.9% 79.9%
=====================================
Files 109 109
Lines 8586 8586
=====================================
+ Hits 6865 6868 +3
+ Misses 1721 1718 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Very nice. |
|
|
||
| - **name**: Unique identifier for your registry | ||
| - **version**: Semantic version of your registry (also called `semconv_version`) | ||
| - **repository_url**: Base URL where schema files are hosted (also called `schema_base_url`) |
There was a problem hiding this comment.
| - **repository_url**: Base URL where schema files are hosted (also called `schema_base_url`) | |
| - **repository_url**: Base URL where schema files are hosted |
not sure if schema_base_url is useful for external readers
There was a problem hiding this comment.
Removed the schema_base_url alias mention. Commit: 19016c3
| name: otel | ||
| description: OpenTelemetry base definitions | ||
| version: 1.0.0 | ||
| repository_url: https://opentelemetry.io/schemas/ |
There was a problem hiding this comment.
| repository_url: https://opentelemetry.io/schemas/ | |
| repository_url: https://github.com/open-telemetry/semantic-conventions | |
| resolved_schema_url: https://opentelemetry.io/schemas/1.42.0 |
(more realistic)
There was a problem hiding this comment.
Updated to use more realistic URLs with resolved_schema_url. Commit: 19016c3
| repository_url: https://acme.com/schemas/ | ||
| dependencies: | ||
| - name: otel | ||
| registry_path: ../otel_registry |
There was a problem hiding this comment.
would it be a realistic/recommended use case?
| registry_path: ../otel_registry | |
| registry_path: https://opentelemetry.io/schemas/1.42.0 |
There was a problem hiding this comment.
Updated to use the resolved schema URL format. Commit: 19016c3
| Add an `imports` section at the top level of your YAML schema file (alongside `groups`): | ||
|
|
||
| ```yaml | ||
| groups: |
There was a problem hiding this comment.
@copilot it's not valid schema v2, update it to use schema v2
attributes:
- key: ...don't forget version: "2"
There was a problem hiding this comment.
Updated all schema examples to use v2 format with version: "2" and attributes with key: instead of id:. Commit: 19016c3
|
|
||
| ### How Imports Work | ||
|
|
||
| When you import a group: |
There was a problem hiding this comment.
| When you import a group: | |
| When you import a signal definition: |
There was a problem hiding this comment.
Updated to say "signal definition" instead of "group". Commit: 19016c3
|
|
||
| When you import a group: | ||
|
|
||
| 1. The group definition from the dependency is included in your resolved schema |
There was a problem hiding this comment.
@copilot "group" is so v1, use attribute or signal definition depending on the context
There was a problem hiding this comment.
Updated terminology throughout - changed "group" to "signal" or "attribute" depending on context. Commit: 19016c3
Co-authored-by: lmolkova <2347409+lmolkova@users.noreply.github.com>
| 2. Any attributes referenced by that group are also included | ||
| 3. The imported groups can be referenced using `ref` in your custom definitions | ||
| 1. The signal definition from the dependency is included in your resolved schema | ||
| 2. Any attributes referenced by that signal are also included |
There was a problem hiding this comment.
| 2. Any attributes referenced by that signal are also included | |
| 2. Any attributes referenced by that definition are also included |
There was a problem hiding this comment.
Updated to use "definition" for consistency. Commit: 90e6523
Co-authored-by: lmolkova <2347409+lmolkova@users.noreply.github.com>
| type: string | ||
| brief: Name of the application. | ||
|
|
||
| spans: |
There was a problem hiding this comment.
@copilot, where did this span come from? it's not quite useful here
There was a problem hiding this comment.
Removed the span example from the application registry. The example now focuses on showing how importing a metric (example.counter) makes its referenced attributes (error.type) available. Commit: 90e6523
| 2. Any attributes referenced by that signal are also included | ||
| 3. The imported signals can be referenced using `ref` in your custom definitions | ||
|
|
||
| ### Example: Referencing Imported Definitions |
There was a problem hiding this comment.
@copilot I don't see an example of referencing imported metric or span, only attribute, but that's trivial
There was a problem hiding this comment.
Updated the example to clearly show metric imports. Added explanation showing how the imported example.counter metric is available in the custom registry and how attributes become accessible through the imported metric. Commit: 90e6523
…e availability Co-authored-by: lmolkova <2347409+lmolkova@users.noreply.github.com>
| description: OpenTelemetry base definitions | ||
| version: 1.0.0 | ||
| repository_url: https://github.com/open-telemetry/semantic-conventions | ||
| resolved_schema_url: https://opentelemetry.io/schemas/1.42.0 |
There was a problem hiding this comment.
@copilot I was wrong, it should be
| resolved_schema_url: https://opentelemetry.io/schemas/1.42.0 | |
| resolved_schema_url: https://opentelemetry.io/schemas/1.42.0/resolved_schema.yaml |
There was a problem hiding this comment.
Updated to include the full path: resolved_schema_url: https://opentelemetry.io/schemas/1.42.0/resolved_schema.yaml. Commit: 0cc50af
| - ref: auction.id | ||
| - ref: auction.name | ||
| - ref: error.type # References attribute from dependency | ||
|
|
There was a problem hiding this comment.
TODO: explain to copilot how to refine imported metric
There was a problem hiding this comment.
The example shows that imported metrics are available in the resolved schema. Refining/extending imported metrics would typically be done by creating new metric definitions in your custom registry that reference the same attributes, rather than modifying the imported metric itself. The current example demonstrates the core import capability.
| **With `--include-unreferenced`:** | ||
| - ✅ All signals and attributes included, even `unused` and `session.end` | ||
|
|
||
| ## Real-World Example: OpenTelemetry Semantic Conventions |
There was a problem hiding this comment.
what does this example add on top of previous ones?
There was a problem hiding this comment.
Removed the repetitive "Real-World Example" section. The registry manifest dependency and attribute referencing are already demonstrated in earlier sections. Commit: 0cc50af
|
|
||
| ## Common Use Cases | ||
|
|
||
| ### Use Case 1: Vendor Extensions |
There was a problem hiding this comment.
@copilot these examples seem to repeat what'd already explained in the doc, maybe just list a few bullet points in the beginning of the doc, no need to repeat yaml again?
There was a problem hiding this comment.
Consolidated the "Common Use Cases" section into brief bullet points without repeating YAML examples. Removed the redundant code examples. Commit: 0cc50af
| weaver registry generate [--include-unreferenced] <registry-path> <template> | ||
|
|
||
| # Validate a registry | ||
| weaver registry check [--include-unreferenced] <registry-path> |
There was a problem hiding this comment.
does it even make sense with --include-unreferenced?
There was a problem hiding this comment.
Simplified the Command Reference section. Removed the detailed --include-unreferenced examples for each command and just noted that most commands support the flag. Commit: 0cc50af
| weaver registry check [--include-unreferenced] <registry-path> | ||
|
|
||
| # Update resolved schema | ||
| weaver registry update-markdown [--include-unreferenced] <registry-path> |
There was a problem hiding this comment.
Simplified the Command Reference section. Removed the detailed --include-unreferenced examples for each command and just noted that most commands support the flag. Commit: 0cc50af
| weaver registry stats [--include-unreferenced] <registry-path> | ||
|
|
||
| # Generate diff between registry versions | ||
| weaver registry diff [--include-unreferenced] <baseline-path> <head-path> |
There was a problem hiding this comment.
does it make sense with --include-unreferenced?
There was a problem hiding this comment.
Simplified the Command Reference section. Removed the detailed --include-unreferenced examples for each command and just noted that most commands support the flag. Commit: 0cc50af
| - Check that archive paths (in `[brackets]`) point to the correct directory inside the archive | ||
| - Ensure the URL points to a registry with a valid `registry_manifest.yaml` | ||
|
|
||
| ## Command Reference |
There was a problem hiding this comment.
is --include-unreferenced important enough that we spend 50% of this doc on it?
There was a problem hiding this comment.
Significantly reduced --include-unreferenced coverage from ~50% to ~10% of the document. Removed the verbose examples and consolidated into a brief explanation with a simple before/after comparison. Reduced total document from 491 to 316 lines (35% reduction). Commit: 0cc50af
…consolidate examples Co-authored-by: lmolkova <2347409+lmolkova@users.noreply.github.com>
| In this mode: | ||
| - ✅ All definitions from dependencies are included, regardless of references | ||
| - ✅ This includes attributes, metrics, events, and entities | ||
| - ⚠️ Results in a larger resolved schema | ||
|
|
||
| ### When to Use Each Mode | ||
|
|
||
| **Use default mode (without flag)** when: | ||
| - You want a minimal schema with only used definitions | ||
| - You're generating code and want to avoid unused definitions | ||
| - You want to reduce the size of your resolved schema | ||
|
|
||
| **Use `--include-unreferenced`** when: | ||
| - You need complete visibility into all available definitions | ||
| - You're exploring or documenting what's available in dependencies | ||
| - You're building tooling that needs to know about all possible definitions | ||
| - You want to generate comprehensive documentation |
There was a problem hiding this comment.
Restored the "When to Use Each Mode" section explaining when to use default mode vs --include-unreferenced. Commit: d12a4d8
| ``` | ||
| **Testing**: Create isolated test registries for validating instrumentation | ||
|
|
||
| ## Best Practices |
There was a problem hiding this comment.
return examples back in this section
There was a problem hiding this comment.
Restored the YAML examples in Common Use Cases section, showing vendor extensions and application-specific metrics with full code samples. Commit: d12a4d8
|
|
||
| # Generate code from a registry | ||
| weaver registry generate [--include-unreferenced] <registry-path> <template> | ||
| weaver registry generate <registry-path> <template> |
There was a problem hiding this comment.
| weaver registry generate <registry-path> <template> | |
| weaver registry generate [--include-unreferenced] <registry-path> <template> |
There was a problem hiding this comment.
Added [--include-unreferenced] to the generate command. Commit: d12a4d8
| **Vendor Registry** (`acme_registry/registry_manifest.yaml`): | ||
| ```yaml | ||
| name: acme | ||
| description: Acme vendor-specific definitions | ||
| version: 0.1.0 | ||
| repository_url: https://acme.com/schemas/ | ||
| dependencies: | ||
| - name: otel | ||
| registry_path: https://opentelemetry.io/schemas/1.42.0 | ||
| ``` |
There was a problem hiding this comment.
@copilot In the current model (where we only support flat hierarchies), it would be better/more realistic to use an example like: OTel <- Company A shared registry <- Application registry (Company A).
There was a problem hiding this comment.
Updated the three-level hierarchy example to use a more realistic scenario: OTel <- Company A shared registry <- Application registry (Company A). Changed vendor "acme" references to "company-a-shared" to better reflect real-world usage patterns. Commit: d12a4d8
Co-authored-by: lquerel <657994+lquerel@users.noreply.github.com>
lquerel
left a comment
lquerel
left a comment
There was a problem hiding this comment.
LGTM. Just made a suggestion that copilot already took into account.
|
closing this and will re-prompt for new docs now that we've added a bunch more fixes in core. |
github-project-automation
Bot
moved this from To consider for the next release
to Done
in OTel Weaver Project
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.