remove specification for service.name #68
Conversation
The attributes should really just be treated like a map of values. Specifying the service name here doesn't provide much value. Additionally, this is preventing the go-jsonschema library from generating the code for attributes as a map. With the service name specification, the generated code looks like this:
```
type Attributes struct {
// ServiceName corresponds to the JSON schema field "service.name".
ServiceName *string `mapstructure:"service.name,omitempty"`
}
```
Without the service name property specified:
```
type Attributes map[string]interface{}
```
I understand this appears to be a bug in go-jsonschema, but after thinking about this, i decided to open this PR as a proposal to work around the issue. Defining the service name property made sense when we wanted to require it, but since it's not required, i dont see much benefit to having it.
Signed-off-by: Alex Boten <aboten@lightstep.com>
| "type": "string" | ||
| } | ||
| } | ||
| "additionalProperties": true |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
correct... i will try to submit a fix for supporting additional properties
There was a problem hiding this comment.
@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
There was a problem hiding this comment.
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.
The attributes should really just be treated like a map of values. Specifying the service name here doesn't provide much value. Additionally, this is preventing the go-jsonschema library from generating the code for attributes as a map. With the service name specification, the generated code looks like this:
Without the service name property specified:
I understand this appears to be a bug in go-jsonschema, but after thinking about this, i decided to open this PR as a proposal to work around the issue. Defining the service name property made sense when we wanted to require it, but since it's not required, i dont see much benefit to having it.