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

Update Instrument validation and move to SDK #1706

Closed
3 tasks
kaylareopelle opened this issue Aug 29, 2024 · 1 comment · Fixed by #1735
Closed
3 tasks

Update Instrument validation and move to SDK #1706

kaylareopelle opened this issue Aug 29, 2024 · 1 comment · Fixed by #1735
Assignees
Labels

Comments

@kaylareopelle
Copy link
Contributor

Instrument name, unit, and version all have certain guidelines to adhere to. Currently, the logic to validate the instrument name is in the API, as part of OpenTelemetry::Metrics::Meter#create_instrument:

def create_instrument(kind, name, unit, description, callback, advice = nil)
raise InstrumentNameError if name.nil?
raise InstrumentNameError if name.empty?
raise InstrumentNameError unless NAME_REGEX.match?(name)
raise InstrumentUnitError if unit && (!unit.ascii_only? || unit.size > 63)
raise InstrumentDescriptionError if description && (description.size > 1023 || !utf8mb3_encoding?(description.dup))
@mutex.synchronize do
OpenTelemetry.logger.warn("duplicate instrument registration occurred for instrument #{name}") if @instrument_registry.include? name
@instrument_registry[name] = yield
end
end

Since the original implementation, the guidance has changed. The logic should now live in the SDK rather than the API.

Move the logic from the SDK to the API and verify that the validation procedures still match the spec.

Copy link
Contributor

👋 This issue has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the keep label to hold stale off permanently, or do nothing. If you do nothing this issue will be closed eventually by the stale bot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants