-
Notifications
You must be signed in to change notification settings - Fork 8
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 attribute linting and ecosystem support. #19
Conversation
I keep repeating the same Elixir code in my reporters, middleware, and unit tests. This commit extracts all that code, cleans it up, and makes it available for everyone else working with Opencensus in Elixir. - **Alter `Opencensus.Trace.with_child_span/2` attribute handling** - Add `Opencensus.Trace.put_span_attributes/1` - Add `Opencensus.Trace.async/1` - Add `Opencensus.Trace.async/3` - Add `Opencensus.Trace.await/1` - Add `Opencensus.Span` struct derived from the record - Add `Opencensus.Span.from/1` - Add `Opencensus.Span.load/1` - Add `Opencensus.SpanContext` struct derived from the record - Add `Opencensus.SpanContext.from/1` - Add `Opencensus.SpanContext.hex_trace_id/1` - Add `Opencensus.SpanContext.hex_span_id/1` - Add `Opencensus.TestSupport.SpanCaptureReporter` - Add `Opencensus.Attributes.process_attributes/1` - Add `Opencensus.Attributes.process_attributes/2` - Add `Opencensus.Attributes.default_attributes/1` The new attribute handling: - **Drops span attributes with unsupported values** - **Flattens nested maps** - Documents the value support limits imposed by the Opencensus wire protocol and `:ocp.put_span_attribute/2` - Preserves the `:module`, `:file`, `:function`, `:line`, and `:default` magic attributes Misc: - `mix credo` - `mix test.watch`
Called out the most controversial parts in bold. Strikes me it's irresponsible to let package developers set span attributes that'll result in their spans getting dumped. Even if we decide that's the job of the reporters, we'll still want to offer the code to get the heavy lifting right. The rest is just useful. |
Codecov Report
@@ Coverage Diff @@
## master #19 +/- ##
==========================================
+ Coverage 95.45% 98.07% +2.62%
==========================================
Files 2 6 +4
Lines 22 52 +30
==========================================
+ Hits 21 51 +30
Misses 1 1
Continue to review full report at Codecov.
|
Can this also include @zachdaniel 's idea to have default attributes be a config option? Instead of all this confusion with a list of default attributes like That or just always include them as we can say "don't use the Elixir macro" if you don't want the extra attributes. |
I made all four compulsory in my internal macro. Checking app config and including them or excluding them by default is a decent enough middle ground. |
Split #20 out; will come back with another PR for attribute control. |
I keep repeating the same Elixir code in my reporters, middleware, and unit tests. This commit extracts all that code, cleans it up, and makes it available for everyone else working with Opencensus in Elixir.
Opencensus.Trace.with_child_span/2
attribute handlingOpencensus.Trace.put_span_attributes/1
Opencensus.Trace.async/1
Opencensus.Trace.async/3
Opencensus.Trace.await/1
Opencensus.Span
struct derived from the recordOpencensus.Span.from/1
Opencensus.Span.load/1
Opencensus.SpanContext
struct derived from the recordOpencensus.SpanContext.from/1
Opencensus.SpanContext.hex_trace_id/1
Opencensus.SpanContext.hex_span_id/1
Opencensus.TestSupport.SpanCaptureReporter
Opencensus.Attributes.process_attributes/1
Opencensus.Attributes.process_attributes/2
Opencensus.Attributes.default_attributes/1
The new attribute handling:
Drops span attributes with unsupported values
Flattens nested maps
Documents the value support limits imposed by the Opencensus wire protocol and
:ocp.put_span_attribute/2
Preserves the
:module
,:file
,:function
,:line
, and:default
magic attributesMisc:
mix credo
mix test.watch