-
Notifications
You must be signed in to change notification settings - Fork 858
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
Follow spec advice on span limits, batch processors #7024
Comments
@jack-berg can I work on this |
Go for it! |
Hi @jack-berg , Hope you are doing good. Just wanted to know, if you are expecting to get any help/general improvement across any part of the repository, I can help you with that. Please do let me know, in-case of any such issue even its a bit difficult, meanwhile I try to find a few... |
Hi thanks @devatbosch - take a look at the help wanted section in the readme and see if you're up for anything discussed there! |
Hey @jack-berg, I created a PR regarding this issue, would you have a chance to take a look at it? |
Why is this a requirement? Wouldn't it be ok to fill up an export batch with multiple queue drains? |
@breedx-splk good point. We'll see what the spec folks say: open-telemetry/opentelemetry-specification#4388 |
Working on #7023 to implement spec PR open-telemetry/opentelemetry-specification#4331, I realized other parts of our implementation are not aligned with the spec:
LogLimitsBuilder
enforce that all the values are positive, when the spec (rightly) says that 0 is an acceptable value.LogLimitsBuilder
has the same issues.BatchLogRecordProcessorBuilder
don't enforce any limits around max queue size. This means its possible to set the queue size to zero or a negative value, which is not allowed in the spec. Not sure what the underlying queue implementations do with a zero or negative value, but we add checks for this. Also, the max export size must be less than or equal to the max queue size.The text was updated successfully, but these errors were encountered: