JSON RPC: define attributes on invalid requests

[louis-cl]

The jsonrpc conventions require the presence of rpc.method.

In some invalid request scenarios, it is impossible to determine the method trying to be invoked.
Like from the jsonrpc specification examples:

--> {"jsonrpc": "2.0", "method": "foobar, "params": "bar", "baz]
<-- {"jsonrpc": "2.0", "error": {"code": -32700, "message": "Parse error"}, "id": null}

Should there be a convention for the telemetry in this scenario?

I can imagine a few options:

1. Don't generate metrics for this scenario
2. Break convention and don't send rpc.method
3. Send a special value for rpc.method like ""
4. Make rpc.method conditionally required

I see value in counting how many jsonrpc errors with code -32700 my server returns.
I would be erring on option 3 to respect the convention.

Extra thoughts on option 3

Why "" ? the jsonspec requires a null on these errors:

If there was an error in detecting the id in the Request object (e.g. Parse error/Invalid Request), it MUST be Null.

And the semantic convention maps it to empty string

Use empty string in case of null value

It felt reasonable to follow the same results for when rpc.method can't be detected.

[thompson-tomo]

@louis-cl why wouldn't your method be foobar? There is nothing specifying that it needs to be a valid in fact using an invalid one would be useful as you could see which methods are generating errors.

[louis-cl]

@louis-cl why wouldn't your method be foobar? There is nothing specifying that it needs to be a valid in fact using an invalid one would be useful as you could see which methods are generating errors.

The JSON request is invalid, notice the lack of " after foobar or the missing }.
If I try the request part from the example I quote {"jsonrpc": "2.0", "method": "foobar, "params": "bar", "baz] in jsonlint.com I get this error:

Error: Parse error on line 1:
..."method": "foobar, "params": "bar", "baz
-----------------------^
Expecting 'EOF', '}', ':', ',', ']', got 'undefined'

Detecting foobar as method would require some smart heuristics in parsing.
Maybe the scenario is clearer with this other invalid request: {"jsonrpc": "2.0", "bar", "baz]

[thompson-tomo]

Ahh, I didn't look that closely at it. Based on the current definition of rpc metrics, they all assume that the message was able to be sent due to the lack of error state attribute. Reading the spec, the reason it is required is that it is needed for the span name but we are talking about metrics here.

What I would propose is make the method attribute required on the span & then remove the attribute from the jsonrpc page as it is the same as the standard definition. That should address the scenario.

[louis-cl]

Adjusting the convention to not require it for metrics works for me, it's akin to option 4 in my first message.