Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Instrumentation queries #1845

Closed
kopeclu2 opened this issue Sep 14, 2023 Discussed in #1829 · 1 comment
Closed

Instrumentation queries #1845

kopeclu2 opened this issue Sep 14, 2023 Discussed in #1829 · 1 comment

Comments

@kopeclu2
Copy link

Discussed in #1829

Originally posted by kopeclu2 August 29, 2023
I'm seeking advice on utilizing Instrumentation and data loaders. I'm encountering an issue when using Instrumentation and data loaders together. The main scenario involves employing multiple data loaders in a data fetcher. However, the problem arises when attempting to use the dispatchIfNeeded(env) method without Instrumentation.

Based on the guidance provided in the Instrumentation DataLoader tutorial, it's clear that the dispatchIfNeeded method is crucial for leveraging multiple loading data loaders. To enable this, I've configured the batching execution strategy by modifying environment properties as follows:

graphql.batching.enabled=true
graphql.batching.strategy=LEVEL_DISPATCHED

This configuration introduces a new DataLoaderLevelDispatchedInstrumentation bean into the GraphQL instance. However, once this is implemented, numerous requests are initiated towards the server involving data loaders and data fetchers. Unfortunately, these requests remain incomplete and time out without being subscribed.

To address this issue, I incorporated an additional instrumentation data loader, DataLoaderDispatcherInstrumentation, which appears to be the default choice. This addition resolved the problem and the system now operates as expected.

Nevertheless, I'm still uncertain about the reasons behind this success. Is this the appropriate setup for GraphQL? Why were the requests failing to execute with only the level dispatched instrumentation? Why was it necessary to include DataLoaderDispatcherInstrumentation?

Here's the relevant code snippet for context:

override fun get(environment: DataFetchingEnvironment): CompletableFuture<SubdomainDataResponse.VideoResponse> {
        val response = environment.getSource<SubdomainDataResponse>()
        val subdomainId = response.id
        val languageId = response.languageId

        return environment.loadOverrideVideo(subdomainId, languageId) // Using first load of data loader, see #1
            .thenCompose { languageEntity ->
                if (languageEntity != null) {
                    mapLanguageEntityToVideoResponse(languageEntity)
                } else {
                    loadDefaultVideoOrENVideo(environment, languageId) // Using a second different data loader, see #2
                }
            }
    }


private fun loadDefaultVideoOrENVideo(
        environment: DataFetchingEnvironment,
        languageId: Language
    ): CompletableFuture<SubdomainDataResponse.VideoResponse> {
        return environment.loadVideoForLanguage(languageId)
            .thenCompose { subdomainDefaultLanguageVideoEntity ->
                if (subdomainDefaultLanguageVideoEntity == null) {
                    environment.loadVideoForLanguage(Language.EN) // Using another data loader call
                        .thenApply { defaultENVideo ->
                            if (defaultENVideo != null) {
                                return@thenApply subdomainDataMapper.mapToVideResponse(defaultENVideo)
                            }
                            throw SubdomainDefaultEnglishVideoDoesNotExistsException()
                        }
                } else {
                    mapDefaultVideoEntityToVideoResponse(subdomainDefaultLanguageVideoEntity)
                }
            }
    }

// #1 calling data loader for the first time in data fetcher, i dont need to call `dispatchIfNeeded`
    private fun DataFetchingEnvironment.loadOverrideVideo(
        subdomainId: UUID,
        languageId: Language
    ): CompletableFuture<SubdomainLanguageVideoEntity> {
        return getDataLoader<SubdomainIdAndLanguageIdKey, SubdomainLanguageVideoEntity>(
            SubdomainLanguageForSubdomainIdDataLoader.name
        ).load(SubdomainIdAndLanguageIdKey(subdomainId, languageId)) // Using load from DataLoader
    }

#2 calling data loader for second time and different dataloader than in #1. I need to use method  `dispatchIfNeeded`
    private fun DataFetchingEnvironment.loadVideoForLanguage(language: Language): CompletableFuture<SubdomainDefaultLanguageVideoEntity> {
        return getDataLoader<Language, SubdomainDefaultLanguageVideoEntity>(
            SubdomainLanguagesDefaultVideosDataLoader.name
        ).load(language) // Using load from DataLoader
            .dispatchIfNeeded(this) // Using method dispatchIfNeeded
    }

Setting up

@ExcludeFromTestCoverage
@Configuration
class InstrumentationConfiguration {

    @Bean
    fun sentryInstrumentation(): Instrumentation {
        return SentryInstrumentation()
    }

    /**
     * - Need to add DataLoaderDispatcherInstrumentation which helps call simple data loaders
     */
    @Bean
    fun dataLoaderDispatcherInstrumentation(): DataLoaderDispatcherInstrumentation {
        return DataLoaderDispatcherInstrumentation()
    }
}

Full query for data loaders:

query ($language: Language!, $subdomainName: String!) {
    getSubdomainData (
        language: $language,
        subdomainName: $subdomainName
    ) {
        __typename
        ... on SubdomainDataResponse {
            city
            companyNameOrPersonalName
            country { 
                code
                currency { 
                    code
                    name
                }
                name
            }
            email
            facebook
            id
            instagram
            language { 
                code
                name
                sort
            }
            name
            phone
            user { 
                email
                firstName
                id
                lastName
                profileImage
            }
            video { 
                videoLink
                videoName
                videoProvider
            }
            youtube
        }
        ... on SubdomainNotFoundByNameErrorResponse {
            message
        }
    }
}

In conclusion, I have a few questions:

Is it possible to employ multiple data loaders within a single data fetcher without utilizing dispatchIfNeeded(env) and Instrumentation?

Why is it mandatory to include both DataLoaderDispatcherInstrumentation and DataLoaderLevelDispatchedInstrumentation for requests and mutations to execute properly? Without the former, most requests fail to execute.

Can I use multiple data loaders in a single data fetcher without enabling global batching?

Is it advisable to simultaneously utilize DataLoaderDispatcherInstrumentation and DataLoaderLevelDispatchedInstrumentation?

I appreciate your insights and guidance. Thank you for your assistance. Have a great day!

@samuelAndalon
Copy link
Contributor

Hello 👋

Is it possible to employ multiple data loaders within a single data fetcher without utilizing dispatchIfNeeded(env) and Instrumentation?

short answer is no, if you need to chain dataloaders in a single resolver, the ability to dispatch those dataloaders is not part of the execution engine anymore (through hooks), thats why the dispatchIfNeeded extension fn exists.

Why is it mandatory to include both DataLoaderDispatcherInstrumentation and DataLoaderLevelDispatchedInstrumentation for requests and mutations to execute properly? Without the former, most requests fail to execute.

is NOT, DataLoaderDispatcherInstrumentation is the dispatching instrumentation provided by graphql-java, which works only for single operations. graphql-kotlin
DataLoaderLevelDispatchedInstrumentation and DataLoaderSyncExecutionExhaustedInstrumentation work with batched operations

curious what version of graphql-kotlin are you using, there is an issue that was fixed couple of months ago #1767 and released in 6.4.1

having said that, if you are interested in using dispatchIfNeeded i would advice to use SYNC_EXHAUSTION batching strategy instead of LEVEL.

if your problem persists, please share a repo link with clear repro instructions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants