parse OTLP endpoint address

[Geal]

Fix #149

the endpoint address is often specified as host:port instead of a full
URL: https://github.com/open-telemetry/opentelemetry-collector/blob/main/exporter/otlpexporter/README.md#getting-started

Unfortunately, the url crate would parse 'collector:4317' as an URL with
the 'collector' scheme and no host. It also will not recognize
'IP:port'.

This adds a layer to URl parsing to recognize those cases, and configure
the HTTPS scheme manually

[o0Ignition0o]

Lgtm, just a question about documentation

[o0Ignition0o]

should we document somewhere that we're falling back to https ?

[Geal]

in https://github.com/open-telemetry/opentelemetry-collector/blob/main/exporter/otlpexporter/README.md#getting-started "By default, TLS is enabled" so it's expected. But I agree that it should be in the documentation.
I'm planning to rewrite the part of the documentation about telemetry once this week's pull requests have settled a bit

[cecton]

You can probably do this trick in the ExportConfig struct directly and skip EndpointUrl struct entirely

[Geal]

I tried various approaches with or without EndpointUrl, like serde(flatten) and those did not work. Unfortunately I do not remember the error messages

[cecton]

#[derive(Debug, Clone, Deserialize, Serialize)]
#[serde(deny_unknown_fields, rename_all = "snake_case")]
pub struct ExportConfig {
    #[serde(deserialize_with = "endpoint_url")]
    pub endpoint: Option<Url>,
    pub protocol: Option<Protocol>,
    pub timeout: Option<u64>,
}

fn endpoint_url<'de, D>(deserializer: D) -> Result<Option<Url>, D::Error>
where
    D: serde::Deserializer<'de>,
{
    let buf = String::deserialize(deserializer)?;
    Ok(Some(Url::parse(&buf).map_err(serde::de::Error::custom)?))
}

[cecton]

With the http fix:

#[derive(Debug, Clone, Deserialize, Serialize)]
#[serde(deny_unknown_fields, rename_all = "snake_case")]
pub struct ExportConfig {
    #[serde(deserialize_with = "endpoint_url")]
    pub endpoint: Option<Url>,
    pub protocol: Option<Protocol>,
    pub timeout: Option<u64>,
}

fn endpoint_url<'de, D>(deserializer: D) -> Result<Option<Url>, D::Error>
where
    D: serde::Deserializer<'de>,
{
    let mut buf = String::deserialize(deserializer)?;
    if !buf.starts_with("http") {
        buf.insert_str(0, "https://");
    }
    Ok(Some(Url::parse(&buf).map_err(serde::de::Error::custom)?))
}

[Geal]

did you test it? That was the first thing I tried. IIRC It complains that it expects to find an endpoint field.
From what I understand, with that annotation, serde-json first looks for an endpoint field, then passes its content to the function that then returns an Option. Instead of recognizing the field as optional

[cecton]

ah yes you need to add default so it can fallback to None before it fails to parse:

    #[serde(deserialize_with = "endpoint_url", default)]

[Geal]

nice! updating the code rn