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 initial options for SDK configuration #14

Merged
merged 2 commits into from
Nov 21, 2022
Merged

Conversation

codeboten
Copy link
Collaborator

These options include disabled and resource.attributes.

Will rebase after #13 is merged

config.yaml Outdated
Comment on lines 19 to 23
service:
# Sets the value of the `service.name` resource attribute
#
# Environment variable: OTEL_SERVICE_NAME
name: "unknown_service"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
service:
# Sets the value of the `service.name` resource attribute
#
# Environment variable: OTEL_SERVICE_NAME
name: "unknown_service"
# Sets the value of the `service.name` resource attribute
#
# Environment variable: OTEL_SERVICE_NAME
service:\name: "unknown_service"

attributes as a flat map of key values for now.

Should consider whether the value's type is unambiguous. For example, if the attribute is key: 10 does 10 map to a string, integer, or float? Alternatively could do something like:

attributes:
  -  key: service.name
      string_value: "unknown_service"
  - key: foo
     int_value: 10

More verbose yes, but also unambiguous which would address issues like this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So yaml supports !!int to force a type, any thoughts on using that instead of having different field identifier for the different types?

I guess having different field identifiers would allow us to support lists/arrays more easily in the future.

Copy link
Collaborator

Choose a reason for hiding this comment

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

So yaml supports !!int to force a type

Didn't realize that!

Suppose that provides an answer for list / arrays as well (i.e. [!!int 10, !!int 15]) but not super pretty. This alleviates most of my concern. Happy enough going with a service.name: value for now. Either way, I'm sure this won't be the last time it comes up 😁

These values include `disabled` and `resource.attributes`.

Signed-off-by: Alex Boten <[email protected]>
@codeboten codeboten merged commit 34d80e1 into main Nov 21, 2022
@codeboten codeboten deleted the codeboten/enabled branch November 21, 2022 23:53
@jack-berg jack-berg mentioned this pull request Dec 5, 2022
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.

2 participants