Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add guidance around use of title and description keywords #157

Merged
merged 2 commits into from
Jan 15, 2025

Conversation

jack-berg
Copy link
Member

Related to #142, which references that "Type title will not change."

I still agree with this, but don't think we should define type titles using the title keyword. Instead, the title should be sourced from the key values withing $defs. For example, given the following from meter_provider.json, the periodic metric reader's title is PeriodicMetricReader. See PR content for reasoning.

   ...
    "$defs": {
        "PeriodicMetricReader": {
            "type": "object",
            "additionalProperties": false,
            "properties": {
                "interval": {
                    "type": ["integer", "null"],
                    "minimum": 0
                },
                "timeout": {
                    "type": ["integer", "null"],
                    "minimum": 0
                },
                "exporter": {
                    "$ref": "#/$defs/PushMetricExporter"
                },
                "producers": {
                    "type": "array",
                    "items": {
                        "$ref": "#/$defs/MetricProducer"
                    }
                }
            },
            "required": [
                "exporter"
            ],
            "title": "PeriodicMetricReader"
        },
   ...

@jack-berg jack-berg requested a review from a team as a code owner January 14, 2025 21:36
@@ -1,7 +1,6 @@
{
"$id": "https://opentelemetry.io/otelconfig/common.json",
"$schema": "https://json-schema.org/draft/2020-12/schema",
"title": "Common",
Copy link
Member Author

Choose a reason for hiding this comment

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

Notably, removing title in top level schema like this, which collect types for re-use but which aren't referenced anywhere in the schema directly tidies up the result of code generation.

Previously, the opentelemetry-java implementation generated a CommonModel type. It wasn't used anywhere, but it still was generated. With this change, its goes away as it should. I could probably find some way to explicitly omit generation for these kind of types, but it is a nice side affect of what seems like a sensible change for other reasons.

Copy link
Member

@marcalff marcalff left a comment

Choose a reason for hiding this comment

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

LGTM.

@jack-berg jack-berg merged commit 041ff25 into open-telemetry:main Jan 15, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants