dmolokanov/appinsights-rs

Trace events cannot be sent due to invalid `severityLevel` field

Closed this issue · 1 comments

Trace events cannot be sent, because the value of the severityLevel field is serialized with an invalid case (lowercase instead of upper). This is due to the #[serde(rename_all = "camelCase")] attribute applied to the generated struct appinsights::contracts::severity_level::SeverityLevel.

Program to repro, which demonstrates that (1) some events can be sent, but (2) every trace event is invalid:

use appinsights::{telemetry::SeverityLevel, TelemetryClient};

fn main() {
    env_logger::init();

    let i_key = "<some-valid-key>";
    let ai = TelemetryClient::new(i_key.into());

    ai.track_event("my event".into());
    ai.track_trace("my verbose".into(), SeverityLevel::Verbose);
    ai.track_trace("my info".into(), SeverityLevel::Information);
    ai.track_trace("my warning".into(), SeverityLevel::Warning);
    ai.track_trace("my error".into(), SeverityLevel::Error);
    ai.track_trace("my critical".into(), SeverityLevel::Critical);

    std::thread::sleep(std::time::Duration::from_secs(10));
}

After adding some more logging to a local copy of the library, we can see the following in the responses to the attempted POST:

[2020-02-13T20:55:58Z TRACE appinsights::transmitter] retry errors: TransmissionItem { index: 1, status_code: 400, message: "107: Field severityLevel on type MessageData contains invalid enum value. Expected: one of 0, 1, 2, 3, 4, Verbose, Information, Warning, Error, Critical, Actual: verbose" }
[2020-02-13T20:55:58Z TRACE appinsights::transmitter] retry errors: TransmissionItem { index: 2, status_code: 400, message: "107: Field severityLevel on type MessageData contains invalid enum value. Expected: one of 0, 1, 2, 3, 4, Verbose, Information, Warning, Error, Critical, Actual: information" }
[2020-02-13T20:55:58Z TRACE appinsights::transmitter] retry errors: TransmissionItem { index: 3, status_code: 400, message: "107: Field severityLevel on type MessageData contains invalid enum value. Expected: one of 0, 1, 2, 3, 4, Verbose, Information, Warning, Error, Critical, Actual: warning" }
[2020-02-13T20:55:58Z TRACE appinsights::transmitter] retry errors: TransmissionItem { index: 4, status_code: 400, message: "107: Field severityLevel on type MessageData contains invalid enum value. Expected: one of 0, 1, 2, 3, 4, Verbose, Information, Warning, Error, Critical, Actual: error" }
[2020-02-13T20:55:58Z TRACE appinsights::transmitter] retry errors: TransmissionItem { index: 5, status_code: 400, message: "107: Field severityLevel on type MessageData contains invalid enum value. Expected: one of 0, 1, 2, 3, 4, Verbose, Information, Warning, Error, Critical, Actual: critical" }

The correct constants occur in the JSON schemas used for codegen. The the problem is in appinsights::contracts::severity_level:

/// Defines the level of severity for the event.
#[derive(Debug, Clone, PartialEq, Serialize)]
#[serde(rename_all = "camelCase")]
pub enum SeverityLevel {
    Verbose,
    Information,
    Warning,
    Error,
    Critical,
}

The severity levels are single words, and so will get serialized to lowercase due to rename_all = "camelCase". This is the right thing for keys, the wrong thing for values.

I'll note that if I manually remove that serde attribute, I can successfully send trace events, and observe them in the Azure Portal. Another solution may involve only changing how the enum variants are serialized as values, compared to their serialization as keys.

I looked at the contract generator, and now I think I know what the "right" fix is. The generator, when generating an enum, should emit a variant attribute #[serde(rename = "<constantName>"), where <constantName> is the constantName value obtained from EnumConstant.name(). It should also not generate #[serde(rename_all = "camelCase")], at least in this circumstance. (edit: I guess it isn't generating that?) This all assumes I am interpreting the schema files correctly.

The problem is that the upstream codegen library does not yet support doing this. See carllerche/codegen#3. There is an open PR for this feature, but the crate is not actively being maintained right now. See: carllerche/codegen#20 (comment)

Since you already keep the generated contract in version control, the quick fix is probably just to remove the existing annotation, and add a big warning comment and regression test. This seems worth doing because trace submission is wholly broken right now. Alternatively, if there isn't a good justification for the pervasive rename_all, maybe that should be removed, instead. I'm happy to throw a PR together, if you like. Then we could close this out and open a separate issue to address enum codegen (pending the new feature in the codegen dependency), since there are probably other mis-generated enums.