opentelemetry-rust icon indicating copy to clipboard operation
opentelemetry-rust copied to clipboard

[semantic-conventions] provide constants for literal string values to support use as match conditions

Open polivbr opened this issue 3 years ago • 4 comments

Currently, the Key values in the semantic conventions crate cannot be used as conditions inside of a match expression because the inner Cow does not derive PartialEq and Eq.

While it would be nice to be able to use the Keys directly as match conditions, I appreciate that moving away from a Cow based implementation isn't feasible.

What I think would be a workable alternative would be to have constants not only for the Keys, but for the the literal strings used to construct the Keys. With those, the string constants could be used as match conditions even if the Keys cannot.

polivbr avatar Dec 29 '21 15:12 polivbr

Could consider having both constants, you should be able to use Key::as_str as a workaround to check equality as well.

jtescher avatar Dec 30 '21 00:12 jtescher

Right now I'm using Key::as_str with a long chain of if/else statements, as it can't be used in a match statement.

polivbr avatar Dec 30 '21 18:12 polivbr

I'm confused by the issue description: first, there is no Key defined in -semantic-conventions, second, Key itself does have derived PartialEq and Eq implementations. If you mean that Key should have an impl PartialEq<str> for Key, that's trivial, but I don't think it solves your issue:

impl PartialEq<str> for Key {
    fn eq(&self, other: &str) -> bool {
        self.0.as_ref() == other
    }
}

fn foo() {
    match Key::new("foo") {
        "foo" => true,
        _ => false,
    };
}

still errors with

error[E0308]: mismatched types
   --> opentelemetry/src/common.rs:109:9
    |
108 |     match Key::new("foo") {
    |           --------------- this expression has type `Key`
109 |         "foo" => true,
    |         ^^^^^ expected struct `Key`, found `&str`

Still, I don't understand your statement that Key::as_str can't be used in match statement, this seems fine:

fn foo() {
    match Key::new("foo").as_str() {
        "foo" => true,
        _ => false,
    };
}

djc avatar Dec 30 '21 22:12 djc

not sure what you mean by there being no Key defined. You even refer to Key in your own examples.

from resource.rs:

use opentelemetry::Key;
...
pub const CLOUD_PROVIDER: Key = Key::from_static_str("cloud.provider");
...

I'm not looking to match the Key against a list of string values, I'm looking to match a string against a list of Keys:

match Key::new(string_from_external_source) {
   resource::SERVICE_VERSION => ...,
   resource::SERVICE_NAME => ...
}

It doesn't matter if Key implements Eq and PartialEq. The internal Cow would also need to derive them, which it doesn't. Here's a related issue from the rust-postgres repo:

https://github.com/sfackler/rust-postgres/issues/756

polivbr avatar Dec 30 '21 22:12 polivbr

@KallDrexx weren't you working on stuff related to this?

hdost avatar Feb 21 '24 07:02 hdost

Isn't this #1334?

djc avatar Feb 21 '24 09:02 djc

Yes it seems so 👍

hdost avatar Feb 21 '24 15:02 hdost