Transforming Bare schema as opposed to wrapping

[santino]

Hey folks,
thanks for your support on the few topics I raised so far.

I'll go straight to the point, I have raised 2 PRs (#1554 and #1624) to introduce bare mode to rename and filter-schema transforms.
Moving forward it would be good to extend this "mode" to other transforms as well so that people can actually opt-into "bare" mode; which would otherwise be difficult when mixed with the default "wrap" mode.

I understand you're going to look into a long-term solution to split schema generation and execution in Mesh, and at that stage, you will probably also look into performing more transforms with "bare" mode by default, as mentioned here.

In the meantime, "bare" mode can and should be added onto existing transforms as proposed with my PRs and I am keen to get this rolling.
Not sure why the PRs feel a bit stuck, maybe you're discussing how to approach this, but I would really appreciate some feedback and update; hence opening this discussion, rather than polluting PR comments.

As a recap, these are top issues that I have with the default "wrap" approach (the list can probably be extended further):

• "Wrap" has an extra runtime cost, because on top of using mapSchema during initialisation; it requires more mapping when receiving requests (applyRequestTransforms) and preparing responses (applyResultTransforms). Even though this extra mapping doesn't traverse the entire schema, it still traverses the document and all nodes available.
  - "Wrap" has a big runtime cost, at least in terms of Big O notation, because each transform rule (not even set of rules) triggers mapShema which loops through all types and fields available across the entire Graph. mapSchema is registered several times for Types, rootTypes, rootFields, objectFields and inputObjectFields. All this heavy mapping is executed at runtime when receiving requests (applyRequestTransforms) and when preparing response (applyResultTransforms). "Bare" mode won't have any of this runtime cost as it doesn't require any mapping at all since transforms are committed during initialisation.
• When using a transform developers don't even know they're opting into "wrap" mode and so their GraphQL schema is actually wrapped several times and they cannot work with the original schema. This means they're not aware of the impact of doing this.
• The above point creates several issues when working with fixed-schema data sources (anything REST basically, which pretty much excludes only native GraphQL sources). If a developer wants to create custom resolvers, they won't have access to the full "fixed" response sent by the data source, and they would be forced to declare selectionSet so that this can be passed to the wrapping schema to whitelist parts of data that are required. This is boilerplating that increases code size and goes against the patterns of working with "fixed-schema" data sources, where the response is dictated by a contract and doesn't have to be declared further to be accessed.

For completeness, these are the potential restrictions for "bare" mode, as detailed in the proposed documentation:

• "bare" doesn't work with data sources that already "speak" GraphQL natively
• "bare" won't work when applying transforms at all-sources (root) level; because at that stage its source would be a merged GraphQL schema (which brings us back to the previous point, really)
• "bare" cannot work well when mixing transforms that supports bare with transforms that don't. This to me is not really a restriction but proof that we should extend "bare" mode to other transforms as well.

As usual, thanks for the time you will want to dedicate to this.

[ardatan]

We aren't planning to make "bare" mode by default because we cannot assume that internal subschemas are returning the whole data not relying on selection set. Wrapping is the safest option for all cases so we keep wrapping is the default option but you can go with 'bare' mode on your own risk :)
Also wrapping happens only once at schema generation phase and the other transformations so it is not that heavy. And I don't think one line "selectionSet" increases the boilerplate that much but instead they know what they get from the parent object.
@dotansimha cc

[santino]

I'm not mentioning going with "bare" as default here and I respect your choice to not do this.

Right now I would just be happy to have an update on introducing "bare" as per my PRs.
Also, I'm touching base on introducing "bare" to other transforms as well to let developers opt-into "bare" mode, which is otherwise difficult if popular transforms don't support this mode.
So far I raised PRs only for rename and filter-schema because those are the ones that I definitely need.

[santino]

wrapping happens only once at schema generation phase

Yes, but during runtime, when receiving requests and preparing responses a wrapped schema forces extra mapping to be performed. And that mapping works by looping through all Types, rootTypes, rootFields, objectFields and inputObjectFields that to check which ones need to be transformed "on-the-fly".
That mapping causes extra loops through the document and nodes available. This is done per each transform rule defined.
This is precisely the additional runtime cost that I am referring to.

I don't think one line "selectionSet" increases the boilerplate that much

Point is that it won't be "one line".
You have to declare a selectionSet on every custom resolver you need; across multiple data sources.
At the end of the day that can be significant chunks of code that need to be validated, tested, and maintained; where instead with "bare" this would simply not be needed.

[yaacovCR]

• "Wrap" has a big runtime cost, at least in terms of Big O notation, because each transform rule (not even set of rules) triggers mapShema which loops through all types and fields available across the entire Graph. mapSchema is registered several times for Types, rootTypes, rootFields, objectFields...

All true, but mapSchema is used within the transformSchema step at startup not on every request.

All this heavy mapping is executed at runtime when receiving requests (applyRequestTransforms) and when preparing response (applyResultTransforms).

also true, but to clarify, these do not trigger mapSchema calls, and only some result transforms require walking the result

This would be O(n = nodes in result), but much of graphql is also O(that same n) so I don’t think it’s big o notation that is issue, just the cost itself.

In terms of stitching, some of the simpler transformations might be able to be improved further to be “prebuilt” within the wrapping schema resolvers, so that might be another Avenue of development.

I make the above comments only with regard to clarifying what’s going on with schema stitching, without really commenting on the right way forward for non Graphql end points...

[santino]

Thanks for taking into account my performance concerns.

All true, but mapSchema is used within the transformSchema step at startup not on every request

Yes, I am aware that mapSchema as a function gets executed only at startup.
But applyRequestTransforms and applyResultTransforms are still looping through the schema even if not using mapSchema.
I just tried to summarise within a single bullet point, but hard to go into detail.

to clarify, these do not trigger mapSchema calls, and only some result transforms require walking the result

Sure, not mapSchema calls, but still looping through the schema though.
As you say, "each node" in the schema is checked and eventually processed; this is the overhead.

much of graphql is also O(that same n)

I understand what you say, but let's be careful.
Normally GraphQL receives a bulk of data and before sending a response maps through it to apply the selection for what has been requested. The "n" in this case only affects the point of preparing a response and it does not require looping through the entire schema again, but "just" reading the document that is generated when receiving the request.

When we apply transforms we introduce more tasks that are un-related to how GraphQL works under the hood and so definitely have a cost that is a complete addition to the "n"s that GraphQL already involves.

My point here is to reduce the additional "n"s as much as possible and the solution I'm offering with "bare" mode addresses precisely this.

[yaacovCR]

U seem to still believe that transformRequest and transformResult methods traverse the entire schema... I don’t think that’s the case. The schema is traversed at startup with caching of relevant data, although the request and sometimes the result in its entirety are traversed. Can u clarify?

[santino]

You're right, they do not traverse the entire schema.
Most likely the code ending up being triggered is the one available here.
In this case, they would loop through the document and so nodes available.

Probably less severe than what I summarised above (will correct); but still happening per each transform rule defined.
Actually, some rules defined in Mesh config will incur in multiple rules set under the hood. F.i. renaming a field causes a new instance of RenameRootFields, RenameObjectFields and RenameInputObjectFields to be registered.

I would still prefer to remove that when possible.

[santino]

PRs for bare mode have been merged and documentation has been introduced.
Thanks for accepting my contributions and tolerating my opinionated view 😅

I'm currently not planning to introduce "bare" mode on more transforms, but if you believe that more popular transforms should have this, or we get feature requests, I will be happy to look into that.

[ardatan]

Thank you for everything! We'd love to have a chat with you if it works for you too :)