Move metric validation from api to sdk

[ymgyt]

Changes

#1264

• move validation for metrics instrument from api to sdk
• include hyphen in instrument names as valid values
• increase instrument name maximum length from 63 to 255 characters

Merge requirement checklist

• CONTRIBUTING guidelines followed
• Unit tests added/updated (if applicable)
• Appropriate CHANGELOG.md files updated for non-trivial, user-facing changes
• Changes in public API reviewed (if applicable)

[codecov]

Codecov Report

Patch coverage is 97.9% of modified lines.

Files Changed	Coverage
opentelemetry/src/metrics/instruments/mod.rs	ø
opentelemetry-sdk/src/metrics/meter.rs	97.9%

📢 Thoughts on this report? Let us know!.

[shaun-cox]

Why prefer a hardcoded 255 and a constant for the error string over a constant for the threshold value defined in the spec and a literal for the string?

[ymgyt]

@shaun-cox @lalitb Thanks for the review!
I defined max length as const, however I could not figure out how to synchronize them with error messages.

[lalitb]

I defined max length as const, however I could not figure out how to synchronize them with error messages.

Not sure if I understand that. What error are you getting ?

[ymgyt]

I have tried the following methods

String

const INSTRUMENT_NAME_MAX_LENGTH: usize = 255;

const INSTRUMENT_NAME_LENGTH: String = format!(
    "instrument name must be less than or equal to {} characters",
    INSTRUMENT_NAME_MAX_LENGTH,
);

error[E0015]: cannot call non-const formatting macro in constants
  --> src/main.rs:21:52
   |
21 |     "instrument name must be less than or equal to {} characters",
   |                                                    ^^
   |
   = note: calls in constants are limited to constant functions, tuple structs and tuple variants
   = note: this error originates in the macro `$crate::__export::format_args` which comes from the expansion of the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)

Box::leak()

const INSTRUMENT_NAME_MAX_LENGTH: usize = 255;

const INSTRUMENT_NAME_LENGTH: &str = Box::leak(
    format!(
        "instrument name must be less than or equal to {} characters",
        INSTRUMENT_NAME_MAX_LENGTH,
    )
    .into_boxed_str(),
);

error[E0015]: cannot call non-const formatting macro in constants
  --> src/main.rs:35:56
   |
35 |         "instrument name must be less than or equal to {} characters",
   |                                                        ^^
   |
   = note: calls in constants are limited to constant functions, tuple structs and tuple variants
   = note: this error originates in the macro `$crate::__export::format_args` which comes from the expansion of the macro `format` (in Nightly builds, run with -Z macro-backtrace for more info)

Lazy<String>

use once_cell::sync::Lazy;

const INSTRUMENT_NAME_MAX_LENGTH: usize = 255;

const INSTRUMENT_NAME_LENGTH: Lazy<String> = Lazy::new(|| {
    format!(
        "instrument name must be less than or equal to {} characters",
        INSTRUMENT_NAME_MAX_LENGTH
    )
});

fn foo() -> &'static str {
    &*INSTRUMENT_NAME_LENGTH
}
fn main() {
    foo();
}

error[E0515]: cannot return value referencing temporary value
  --> src/main.rs:13:5
   |
13 |     &*INSTRUMENT_NAME_LENGTH
   |     ^^----------------------
   |     | |
   |     | temporary value created here
   |     returns a value referencing data owned by the current function

If I can use Cow as follows for MetricsError , const can be used in error message.
but this would result in breaking change

pub enum MetricsError {
    // ...
    #[error("Invalid instrument configuration: {0}")]
-   InvalidInstrumentConfiguration(&'static str),
+   InvalidInstrumentConfiguration(Cow<'static, str>),
}

fn validate() -> MetricsError {
    MetricsError::InvalidInstrumentConfiguration(Cow::Owned(format!(
        "instrument name must be less than or equal to {} characters",
        INSTRUMENT_NAME_MAX_LENGTH
    )))
}

[TommyCpp]

Don't think it's worth the effort to format a constant string from another constant string. We keep them together and that should be good.

Thanks for working on it.