Promote smile encoding to be preferred over json response encoding

[carterkozak]

Note that this change intentionally does not promote CBOR, which
may be supported on other non-java platforms. This allows us to
minimally take advantage of binary encoding without risking changes
in nonstandard consumers yet.

After this PR

==COMMIT_MSG==
Promote smile encoding to be preferred over json response encoding
==COMMIT_MSG==

Possible downsides?

It's new and different!

[changelog-app]

Generate changelog in changelog/@unreleased

Type

• Feature
• Improvement
• Fix
• Break
• Deprecation
• Manual task
• Migration

Description

Promote smile encoding to be preferred over json response encoding

Check the box to generate changelog(s)

• Generate changelog entry

[ferozco]

Servers are already configured to respond with Smile right? I think I would like to wait for more roll out the client with the current behaviour before switching over the default

[carterkozak]

Sure, we can sit on this until we're ready, but it would be helpful to have a clear definition of when that will be.

[stale]

This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days.

[carterkozak]

Bump

…

On Apr 13, 2020, at 3:10 PM, stale[bot] ***@***.***> wrote:  This PR has been automatically marked as stale because it has not been touched in the last 14 days. If you'd like to keep it open, please leave a comment or add the 'long-lived' label, otherwise it'll be closed in 7 days. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[iamdanfox]

Capturing the plan we discussed in the standup today: wait until we've enabled the feign shim by default in WC (as that will switch most clients to dialogue, but they won't get this fancy decoding), then merge this which will give people an extra incentive to move from feign shim -> codgen.

[carterkozak]

@iamdanfox Are we ready to turn this on?

[iamdanfox]

So I think the main thing I want to avoid is any possibility of a mega fleetwide P0. In practise I think that risk is pretty small here, because people's dockerized tests should stop this merging if smile encoding somehow breaks, and if they don't have dockerized tests then adjudication should roll them back. It seems like we're not in danger of this spontaneuously breaking in prod. It is a little sad that our exhaustive conjure-verification tests will only cover JSON here, but i guess we can get some confidence from gotham's success with smile.

Then there's the question of tracking the effect of this change. I know we've got params.content-type available in Aries, do we also want some instrumentation so we can track fleet-wide prevalance / response times in DD?

(Also it's probably worth mentioning the new behaviour in the README, possibly even giving people the curl equivalents so they can see for themselves what happens if you ask for smile/cbor?)

[ferozco]

Thoughts on changing the weights in WC instead of dialogue then cutting an RC or exposing a temporary flag so that we can have a bit more of controlled rollout?

[sfackler]

Out of curiosity, why is smile specifically the choice here? It appears to be a thing created by the Jackson developers and the level of support seems to be orders of magnitude smaller than other formats like CBOR.

[carterkozak]

We're recording metrics as server.encoding.request and server.encoding.response. Docs internally at witchcraft/blob/develop/witchcraft-core/metrics.md#serverencoding

For what it's worth we've already rolled this out in our largest product successfully for the past year or two. How would rollout differ from the standard library upgrade, and at what point would it become default enabled?

[carterkozak]

@sfackler CBOR is more widely supported but underperformant. In a large internal project we saw a similar benefit moving from JSON to CBOR and CBOR to SMILE.

[iamdanfox]

I think I'm game to give this a shot. It'll only affect pure dialogue clients (so all the feign shim ones won't get this), and adjudication should save us if something goes wrong.

Would still like a few sentences about this negotiation on the README though, especially because the tradeoffs are a little subtle (e.g. gzip achieves a similar response size, but SMILE was preferred for lower compression CPU iirc?)

[svc-autorelease]

Failed to load project - please reach out to #dev-foundry-infra or check Aries to debug