-
Notifications
You must be signed in to change notification settings - Fork 35
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
O11Y-1631: add SpanLimits, implement maxNumAttributes and maxNumAttributeLength #42
Conversation
Merge Requirements Met ✅Request Rosie to automerge this pull request by including @Workiva/release-management-p in a comment. General InformationTicket(s): Code Review(s): #42 Reviewers: changliu-wk, blakeroberts-wk Additional InformationWatchlist Notifications: None
Note: This is a shortened report. Click here to view Rosie's full evaluation. |
Security InsightsNo security relevant content was detected by automated scans. Action Items
Questions or Comments? Reach out on Slack: #support-infosec. |
Public API ChangesRecommendation: @@ line 12: package:opentelemetry/src/api/trace/nonrecording_span.dart @@
class NonRecordingSpan implements Span
- Attributes attributes
// Removing a field is a major change.
+ void setAttribute(Attribute attribute)
// Adding a method is a minor change.
+ void setAttributes(List<Attribute> attributes)
// Adding a method is a minor change.
@@ line 11: package:opentelemetry/src/api/trace/sampling_result.dart @@
abstract class SamplingResult
- Attributes get spanAttributes
+ List<Attribute> get spanAttributes
// `type` of `spanAttributes` has changed from `Attributes` to `List<Attribute>`.
// Changing a class field is a major change.
- SamplingResult SamplingResult(Decision decision, Attributes spanAttributes, TraceState traceState)
+ SamplingResult SamplingResult(Decision decision, List<Attribute> spanAttributes, TraceState traceState)
// `type` of `spanAttributes` has changed.
// Changing a parameter signature is a major change.
@@ line 9: package:opentelemetry/src/sdk/trace/samplers/always_on_sampler.dart @@
class AlwaysOnSampler implements Sampler
- SamplingResult shouldSample(Context context, TraceId traceId, String spanName, bool spanIsRemote, Attributes spanAttributes)
+ SamplingResult shouldSample(Context context, TraceId traceId, String spanName, bool spanIsRemote, List<Attribute> spanAttributes)
// `type` of `spanAttributes` has changed.
// Changing a parameter signature is a major change.
@@ line 6: package:opentelemetry/src/sdk/trace/span.dart @@
class Span implements Span
- Span Span(String name, SpanContext _spanContext, SpanId _parentSpanId, List<SpanProcessor> _processors, Resource _resource, InstrumentationLibrary _instrumentationLibrary, {Attributes attributes})
+ Span Span(String name, SpanContext _spanContext, SpanId _parentSpanId, List<SpanProcessor> _processors, Resource _resource, InstrumentationLibrary _instrumentationLibrary, {List<Attribute> attributes, SpanLimits spanlimits})
// `type` of `attributes` has changed.
// Changing a parameter signature is a major change.
- Attributes attributes
+ Attributes get attributes
// Removed setter for `attributes`.
// Removing a field is a major change.
+ void setAttributes(List<Attribute> attributes)
// Adding a method is a minor change.
+ void setAttribute(Attribute attribute)
// Adding a method is a minor change.
+ int get droppedAttributes
// Adding a field is a minor change.
@@ line 11: package:opentelemetry/src/api/trace/span.dart @@
abstract class Span
- Attributes attributes
// Removing a field is a major change.
+ void setAttributes(List<Attribute> attributes)
// Adding abstract members breaks all subclasses.
+ void setAttribute(Attribute attribute)
// Adding abstract members breaks all subclasses.
Click to see 9 more API Changes
@@ line 5: package:opentelemetry/src/sdk/trace/tracer.dart @@
class Tracer implements Tracer
- Span startSpan(String name, {Context context, Attributes attributes})
+ Span startSpan(String name, {Context context, List<Attribute> attributes})
// `type` of `attributes` has changed.
// Changing a parameter signature is a major change.
- Tracer Tracer(List<SpanProcessor> _processors, Resource _resource, Sampler _sampler, IdGenerator _idGenerator, InstrumentationLibrary _instrumentationLibrary)
+ Tracer Tracer(List<SpanProcessor> _processors, Resource _resource, Sampler _sampler, IdGenerator _idGenerator, InstrumentationLibrary _instrumentationLibrary, {SpanLimits spanLimits})
// `spanLimits` was added.
// Adding an optional parameter is a minor change.
@@ line 4: package:opentelemetry/src/sdk/trace/samplers/sampling_result.dart @@
class SamplingResult implements SamplingResult
- Attributes get spanAttributes
+ List<Attribute> get spanAttributes
// `type` of `spanAttributes` has changed from `Attributes` to `List<Attribute>`.
// Changing a class field is a major change.
- SamplingResult SamplingResult(Decision decision, Attributes spanAttributes, TraceState traceState)
+ SamplingResult SamplingResult(Decision decision, List<Attribute> spanAttributes, TraceState traceState)
// `type` of `spanAttributes` has changed.
// Changing a parameter signature is a major change.
@@ line 5: package:opentelemetry/src/api/trace/sampler.dart @@
abstract class Sampler
- SamplingResult shouldSample(Context context, TraceId traceId, String spanName, bool spanIsRemote, Attributes spanAttributes)
+ SamplingResult shouldSample(Context context, TraceId traceId, String spanName, bool spanIsRemote, List<Attribute> spanAttributes)
// `type` of `spanAttributes` has changed.
// Changing a parameter signature is a major change.
@@ line 4: package:opentelemetry/src/sdk/trace/samplers/parent_based_sampler.dart @@
class ParentBasedSampler implements Sampler
- SamplingResult shouldSample(Context context, TraceId traceId, String spanName, bool spanIsRemote, Attributes spanAttributes)
+ SamplingResult shouldSample(Context context, TraceId traceId, String spanName, bool spanIsRemote, List<Attribute> spanAttributes)
// `type` of `spanAttributes` has changed.
// Changing a parameter signature is a major change.
@@ line 4: package:opentelemetry/src/sdk/trace/samplers/always_off_sampler.dart @@
class AlwaysOffSampler implements Sampler
- SamplingResult shouldSample(Context context, TraceId traceId, String spanName, bool spanIsRemote, Attributes spanAttributes)
+ SamplingResult shouldSample(Context context, TraceId traceId, String spanName, bool spanIsRemote, List<Attribute> spanAttributes)
// `type` of `spanAttributes` has changed.
// Changing a parameter signature is a major change.
@@ line 7: package:opentelemetry/src/api/trace/tracer.dart @@
abstract class Tracer
- Span startSpan(String name, {Context context, Attributes attributes})
+ Span startSpan(String name, {Context context, List<Attribute> attributes})
// `type` of `attributes` has changed.
// Changing a parameter signature is a major change.
@@ line 5: package:opentelemetry/src/sdk/trace/tracer_provider.dart @@
class TracerProvider implements TracerProvider
- TracerProvider TracerProvider({List<SpanProcessor> processors, Resource resource, Sampler sampler, IdGenerator idGenerator})
+ TracerProvider TracerProvider({List<SpanProcessor> processors, Resource resource, Sampler sampler, IdGenerator idGenerator, SpanLimits spanLimits})
// `spanLimits` was added.
// Adding an optional parameter is a minor change.
@@ line 1: package:opentelemetry/src/sdk/trace/span_limits.dart @@
+ class SpanLimits
// Adding a class is a minor change.
@@ line 4: package:opentelemetry/src/api/common/attributes.dart @@
class Attributes
+ int get length
// Adding a field is a minor change.
Showing results for 93030c4
Last edited UTC Apr 13 at 15:53:05 |
d06c0e2
to
100448a
Compare
100448a
to
94ef370
Compare
@changliu-wk This pull request has merge conflicts, please resolve. |
QA +1 all test cases passed and tested in TestApp. |
@Workiva/release-management-p |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 from RM
Problem
NR has a maximum limit on attribute key and value lengths. These result in alerts that fire in our #new-relic-alerts channel. e.g. https://workiva.slack.com/archives/C014FKLGSDP/p1647615771469009
Other language SDKs add a configuration option to limit the total attribute size. Anything above that size will be automatically truncated (see supporting information).
New Relic limits the:
length of attribute keys (unable to find limit)
attribute value size = 1KB (10^3 bytes)
number of attributes per span = 254
Supporting Information
https://docs.newrelic.com/docs/data-apis/manage-data/view-system-limits/
open-telemetry/opentelemetry-java#1484
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/sdk.md#span-limits
Solution
@release-management-p