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.

Sign up for GitHub

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

Jump to bottom

remove specification for service.name #68

Merged
merged 2 commits into from
Mar 1, 2024
Merged

remove specification for service.name #68

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1cb0ea1
remove specification for service.name
Jan 18, 2024
6809943
Merge branch 'main' into codeboten/remove-specification-for-service-name
Jan 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions schema/resource.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,7 @@
"Attributes": {
"title": "Attributes",
"type": "object",
"additionalProperties": true,
"properties": {
"service.name": {
"type": "string"
}
}
"additionalProperties": true
Copy link
Member

Choose a reason for hiding this comment

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

Just so I understand - does the go implementation struggle with all places where we set additionalProperties: true? I think being able to read additional properties will be critical to supporting custom extension components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

correct... i will try to submit a fix for supporting additional properties

Copy link
Contributor Author

@codeboten codeboten Jan 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@jack-berg to be clear I'm not suggesting that we remove support for additional properties across the schema. Running into this issue in go-jsonschema made me wonder if attributes needed any properties defined or not, this is why I opened a PR to remove the service name

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha. Yeah the "required" feels wrong to me. While an SDK is required to produce a resource with service.name, there is no requirement that it is specified by OTEL_SERVICE_NAME or the user, since there is a well defined fallback value.

I think the same should probably apply here: drop the service.name requirement, but heavily encourage in the example documentation.

}
}
}
Loading