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

Add support for array-valued attributes #160

Closed
fbogsany opened this issue Dec 19, 2019 · 3 comments · Fixed by #185
Closed

Add support for array-valued attributes #160

fbogsany opened this issue Dec 19, 2019 · 3 comments · Fixed by #185
Assignees
@fhwang
Milestone
Alpha v0.5

Comments

@fbogsany
Copy link
Contributor

See open-telemetry/opentelemetry-specification#368
@fbogsany fbogsany added this to the Alpha v0.4 milestone Dec 19, 2019
@fhwang
Copy link
Contributor

fhwang commented Feb 14, 2020

As long as I'm looking at this: There seem to be a number of places where the type of attributes is not being validated, for example https://github.com/open-telemetry/opentelemetry-ruby/blob/master/api/lib/opentelemetry/trace/event.rb#L39

Should I add validation in these places, raising ArgumentError in cases where attributes is invalid?

@mwear
Copy link
Member

mwear commented Feb 14, 2020

Having the observability system raise runtime exceptions is frowned upon see: https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/error-handling.md. The preferred approach is to use defaults or discard data that doesn't conform and log a message about it.

@mwear
Copy link
Member

mwear commented Feb 14, 2020

Another trend you may notice, is that validating data can sometimes be expensive to do up front, and since data is sampled, it often is more efficient to validate, drop or correct data on export.

fhwang added a commit to fhwang/opentelemetry-ruby that referenced this issue Feb 15, 2020
@fhwang
Add support for array-valued attributes.
5cbcad8 
Resolves open-telemetry#160.
@fhwang fhwang mentioned this issue Feb 15, 2020
Merged
fbogsany pushed a commit that referenced this issue Mar 10, 2020
@fhwang
Add support for array-valued attributes. (#185)
7f583df 
* Add support for array-valued attributes.

Resolves #160.

* Update documentation of Trace::Span#add_event.

* Test that Trace::Span#add_event can handle empty array values.

* Optimize SDK::Internal::valid_array_value?

* Rename SDK::Internal::valid_primitive_value? to valid_simple_value?

* Tiny change to overcome github.com issue.

* Extract Jaeger::Exporter::SpanEncoder class.

* Add tests for Jaeger::Exporter::SpanEncoder.

* Jaeger::Exporter::SpanEncoder handles array attributes.

* Fix rubocop issues.

* Reduce complexity of SpanEncoder#encoded_tags.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@fhwang fhwang

Labels
None yet
Projects
None yet
Milestone
Alpha v0.5
Development

Successfully merging a pull request may close this issue.

Add support for array-valued attributes. fhwang/opentelemetry-ruby
3 participants
@fhwang @fbogsany @mwear