newrelic / infra-integrations-sdk

New Relic Infrastructure Integrations SDK
Apache License 2.0
46 stars 24 forks source link

Feature Request: Provide Standardized JSON Encoder/Decoder #207

Closed icpenguins closed 2 years ago

icpenguins commented 4 years ago

Dozens of OHIs have to manage JSON Decoding

Over the course of the last 3 weeks, there have been a couple high severity issues with both decoding JSON responses and Marshalling them. Specific commits/issues like the following demonstrate the need to have a more standardized method for managing JSON responses.

Examples

Details

The issues listed above have created critical situations where data was not being reported from the servers/nodes. The larger issue being, unless an actual person is monitoring the data streams, there is often no way to know that these issues are occurring.

Although some marshaling is managed by the SDK, a method to allow all OHIs the ability to benefit from one standardized encoding, decoding, unmarshaling, and marshaling solution could reduce the bug-tail that is currently being seen in production sites. Some of this occurs because the expected interfaces, as defined from the software, can randomly return these, JSON valid, but inappropriate types to convert.

OS
mariomac commented 4 years ago

To be fair, +Inf or undefined are not standard JSON values [1], but we understand the necessity of making our SDK compatible with those services that may return non-standard JSON implementations.

Let me forward this issue to our team and we'll prioritize this.

[1] https://www.json.org/json-en.html

icpenguins commented 4 years ago

@mariomac when I looked at the site you shared, I understand what your point is about the JSON format itself. However, in the ECMA-404 PDF, Infinity is referenced in section 8. It would be my assumption that undefined is not listed in this standard as it is passed as a string rather than a value. It would be the burden of the decoder to know the type of the property to correctly parse the value.

In ECMAScript 5.1 undefined is a primitive along with infinity. Here are the references to both of these values in the ECMAScript 5.1 spec. Both will have to be marshaled at some point to a JSON object if part of an API call.

ECMAScript Language Spec 5.1

4.3.9 undefined value

primitive value used when a variable has not been assigned a value

4.3.22 Infinity

number value that is the positive infinite Number value

mariomac commented 4 years ago

Thanks for the link.

davidgit commented 2 years ago

We appreciate you taking the time to bring this to our attention. After reviewing this issue, we are unable to commit on a timeline to resolution for more than year. When committing to resolve an issue, we take into account the type of issue, the customer impact, and the development effort required. If the situation changes, we will revisit this decision.

We encourage community contributions and are happy to collaborate on design proposals and pull requests. See our PR guidelines in the README. If you have any further questions or comments about this, please let us know by replying to this issue.