Added Weaver configuration for all telemetry, and a generated mkdocs site#2794
Added Weaver configuration for all telemetry, and a generated mkdocs site#2794martinjt wants to merge 22 commits intoopen-telemetry:mainfrom
Conversation
github-actions
bot
added
the
helm-update-required
|
@martinjt can we configure live-check to ensure the telemetry schema is always up to date? I just don't want that to be another burden for maintainers. Also, the k8s manifests shouldn't be part of this PR. That is generated via helm update whenever we have a new release |
|
We can, i just didnt want to make it part of this PR since its already got a lot of moving parts. I can remove the k8s bits. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
@martinjt I had a chat with @puckpuck in the last SIG meeting and we were discussing this PR. Shouldn't we keep this PR just with the Something like how the semconv docs is currently done. |
|
My view is that we need to show people how to use Weaver for the things it's built for (Markdown documentation, and telemetry validation). The easiest of those to see is generating markdown. We show them how that's done in this repo, not be having another repo generate them and display them. If this repo had to have something else host the docs, that wouldn't be something that people could clone, run and iterate on. We need to show people how they can use weaver, give them a place to test it, just like we do with all the languages and their instrumentations. It's totally valid that the otel docs could ALSO use the schema to generate some more docs that are static and updated on a specific schedule. The repo, the helm chart and the images, should be able to run to allow the user to see all of them together, it shouldn't be that I run the demo, then consult the otel website for the docs (which could be for a different version). As an application developer, I would likely have a way of viewing the changes of the documentation, I'd maybe have something in my pipeline generate github pages. Either way, it's local to the repo and something that a developer can generate |
|
What if we keep the doc service as a different docker compose file? This would allow users to test weaver, but the docs services wouldn't always start with the Demo. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
@martinjt I've touched your PR, hope you don't mind.
This should be good to go, but as I've changed a couple of things, I'll wait for your review. |
|
Hey @martinjt just a kind bump 😊 |
| flagd-ui: | ||
| condition: service_started | ||
| telemetry-docs: | ||
| condition: service_started |
There was a problem hiding this comment.
Since the envoy template is shared with minimal docker compose. We should add these in minimal compose or the envoy template will fail to resolve ${TELEMETRY_DOCS_HOST} and ${TELEMETRY_DOCS_PORT}.
| ## Schema Files | ||
|
|
||
| - `telemetry-schema/registry_manifest.yaml` - Registry metadata | ||
| - `telemetry-schema/attributes/common.yaml` - Logical attribute group definitions |
There was a problem hiding this comment.
We don't have common.yaml, Should we remove this?
| - id: app.payment.amount | ||
| type: string | ||
| brief: Payment amount | ||
| stability: stable | ||
| note: The monetary amount being charged | ||
| examples: ["29.99", "99.95", "150.00"] |
There was a problem hiding this comment.
Nice, this helped me to easily see a difference in instrumentation. app.payment.amount in order.yaml is type: string but app.shipping.amount is type: double` amounts should consistently use the same type. Probably needs fix in a separate PR. I already find this useful 👍🏽
| - id: app.shipping.cost.total | ||
| type: string | ||
| brief: Total shipping cost | ||
| stability: stable | ||
| note: The total cost of shipping for an order (string representation) | ||
| examples: ["10.50", "25.00"] |
There was a problem hiding this comment.
Same inconsistency for cost totals. app.shipping.cost.total is type: string while app.quote.cost.total is type: double
| site_name: OpenTelemetry Demo Telemetry Schema | ||
| site_description: Semantic conventions and telemetry schema for the OpenTelemetry Demo application | ||
| site_author: OpenTelemetry Demo Contributors | ||
| site_url: https://opentelemetry-demo.example.com/telemetry/ |
There was a problem hiding this comment.
Should we change this to http://localhost:8080/telemetry/
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
4 participants
Changes
This adds a new service called
telemetry-docswhich serves a documentation site for the custom telemetry documented by the demo across all the services.It also includes a weaver registry for the all those attributes.
This does not include a live-check interface, that will come later, nor does this include codegen for these attributes or metrics.
Merge Requirements
For new features contributions, please make sure you have completed the following
essential items:
CHANGELOG.mdupdated to document new feature additionsMaintainers will not merge until the above have been completed. If you're unsure
which docs need to be changed ping the
@open-telemetry/demo-approvers.