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

chart(feat): add graphql metrics exporter for monitoring #2425

Merged
merged 1 commit into from
Oct 10, 2024

Conversation

VietND96
Copy link
Member

@VietND96 VietND96 commented Oct 9, 2024

User description

Thanks for contributing to the Docker-Selenium project!
A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines, applied for this repository.
Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

  • Add a GraphQL exporter listen on Grid GraphQL endpoint
  • Add additional scrape config for Prometheus to scrape metrics from the GraphQL exporter

Metrics on Prometheus start with prefix query_ and following is graphql schema. It looks like

image

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • I have read the contributing document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

PR Type

Enhancement, Tests


Description

  • Added a GraphQL metrics exporter for monitoring Selenium Grid.
  • Updated Selenium test setup to conditionally add browser options.
  • Enhanced Kubernetes templates with monitoring exporter configurations.
  • Updated Go version and Docker base image for improved compatibility.
  • Added documentation and badges to README for better visibility.

Changes walkthrough 📝

Relevant files
Enhancement
10 files
chart_setup_env.sh
Update Go version in chart setup script                                   

tests/charts/make/chart_setup_env.sh

  • Updated Go version from 1.22.3 to 1.23.2.
+1/-1     
_helpers.tpl
Add GraphQL URL for monitoring exporter                                   

charts/selenium-grid/templates/_helpers.tpl

  • Added GraphQL URL definition for internal monitoring exporter.
+7/-0     
_nameHelpers.tpl
Define fullname for Selenium metrics exporter                       

charts/selenium-grid/templates/_nameHelpers.tpl

  • Defined fullname for Selenium metrics exporter.
+7/-0     
Dockerfile
Add executable permission for websockify script                   

NodeBase/Dockerfile

  • Added executable permission for websockify run script.
+1/-0     
selenium-grid.yaml
Add scrape configuration for Selenium Grid analytics         

charts/selenium-grid/configs/scrape/selenium-grid.yaml

  • Added scrape configuration for Selenium Grid analytics.
+15/-0   
monitoring-exporter-deployment.yaml
Add deployment template for monitoring exporter                   

charts/selenium-grid/templates/monitoring-exporter-deployment.yaml

  • Added deployment template for monitoring exporter.
+35/-0   
monitoring-exporter-service.yaml
Add service template for monitoring exporter                         

charts/selenium-grid/templates/monitoring-exporter-service.yaml

  • Added service template for monitoring exporter.
+27/-0   
monitoring-scape-secret.yaml
Add secret template for monitoring scrape configuration   

charts/selenium-grid/templates/monitoring-scape-secret.yaml

  • Added secret template for monitoring scrape configuration.
+29/-0   
patch-keda-objects-job.yaml
Update KEDA patch job for TriggerAuthentication                   

charts/selenium-grid/templates/patch-keda/patch-keda-objects-job.yaml

  • Updated KEDA patch job to include TriggerAuthentication.
+2/-2     
trigger-auth.yaml
Add labels to trigger authentication template                       

charts/selenium-grid/templates/trigger-auth.yaml

  • Added labels to trigger authentication template.
+2/-0     
Tests
1 files
__init__.py
Enhance browser options setup in Selenium tests                   

tests/SeleniumTests/init.py

  • Conditional addition of Chrome and Edge options for disabling
    features.
  • Added Firefox options for enabling downloads.
  • +9/-6     
    Dependencies
    1 files
    Dockerfile
    Update base image version in Dockerfile                                   

    Base/Dockerfile

  • Updated base image from ubuntu:noble-20240827.1 to
    ubuntu:noble-20240904.1.
  • +1/-1     
    Configuration changes
    1 files
    Makefile
    Update KEDA based tag version                                                       

    Makefile

    • Updated KEDA based tag version.
    +1/-1     
    Documentation
    2 files
    README.md
    Add release downloads badge to README                                       

    README.md

    • Added badge for release downloads.
    +1/-0     
    CONFIGURATION.md
    Update configuration for monitoring and RBAC roles             

    charts/selenium-grid/CONFIGURATION.md

  • Updated RBAC role settings for KEDA resources.
  • Added configuration for monitoring exporter.
  • +18/-2   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    qodo-merge-pro bot commented Oct 9, 2024

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Conditional Logic
    The changes introduce conditional logic for adding browser options. This needs to be reviewed to ensure it works as intended for all browser types.

    New Component
    A new monitoring exporter deployment has been added. This needs to be thoroughly reviewed for security and performance implications.

    KEDA Patch Update
    The KEDA patch job has been updated to include TriggerAuthentication. This change needs to be verified for correct functionality and potential security implications.

    Copy link

    qodo-merge-pro bot commented Oct 9, 2024

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Enhancement
    Extract common browser configuration logic into a separate function to reduce duplication

    Consider extracting the common configuration logic for Chrome and Edge options into
    a separate function to reduce code duplication.

    tests/SeleniumTests/init.py [130-137]

    -options = ChromeOptions()
    -options.enable_downloads = SELENIUM_ENABLE_MANAGED_DOWNLOADS
    -if not SELENIUM_ENABLE_MANAGED_DOWNLOADS:
    -    options.add_argument('disable-features=DownloadBubble,DownloadBubbleV2')
    -if TEST_ADD_CAPS_RECORD_VIDEO:
    -    options.set_capability('se:recordVideo', True)
    -options.set_capability('se:name', f"{self._testMethodName} ({self.__class__.__name__})")
    -options.set_capability('se:screenResolution', '1920x1080')
    +def configure_browser_options(options):
    +    options.enable_downloads = SELENIUM_ENABLE_MANAGED_DOWNLOADS
    +    if not SELENIUM_ENABLE_MANAGED_DOWNLOADS:
    +        options.add_argument('disable-features=DownloadBubble,DownloadBubbleV2')
    +    if TEST_ADD_CAPS_RECORD_VIDEO:
    +        options.set_capability('se:recordVideo', True)
    +    options.set_capability('se:name', f"{self._testMethodName} ({self.__class__.__name__})")
    +    options.set_capability('se:screenResolution', '1920x1080')
    +    return options
     
    +options = configure_browser_options(ChromeOptions())
    +
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Extracting common configuration logic into a separate function reduces code duplication and enhances maintainability. This is a beneficial improvement, especially if the configuration logic is used in multiple places.

    7
    Maintainability
    Improve readability of kubectl commands by using multi-line YAML blocks

    Consider using a multi-line YAML block for the command arguments to improve
    readability and maintainability.

    charts/selenium-grid/templates/patch-keda/patch-keda-objects-job.yaml [35-39]

     args:
       - |
         echo "Cleaning up ScaledObjects, ScaledJobs and HPAs for {{ .Release.Name }} when upgrading or disabling autoscaling."
    -    kubectl get ScaledObjects,ScaledJobs,TriggerAuthentication -n {{ .Release.Namespace }} -l component.autoscaling={{ .Release.Name }} -o=json | jq '.metadata.finalizers = null' | kubectl apply -f - || true ;
    -    kubectl delete ScaledObjects,ScaledJobs,TriggerAuthentication -n {{ .Release.Namespace }} -l component.autoscaling={{ .Release.Name }} --wait || true ;
    -    kubectl delete hpa -n {{ .Release.Namespace }} -l component.autoscaling={{ .Release.Name }} --wait || true ;
    +    kubectl get ScaledObjects,ScaledJobs,TriggerAuthentication \
    +      -n {{ .Release.Namespace }} \
    +      -l component.autoscaling={{ .Release.Name }} \
    +      -o=json | jq '.metadata.finalizers = null' | kubectl apply -f - || true
    +    kubectl delete ScaledObjects,ScaledJobs,TriggerAuthentication \
    +      -n {{ .Release.Namespace }} \
    +      -l component.autoscaling={{ .Release.Name }} \
    +      --wait || true
    +    kubectl delete hpa \
    +      -n {{ .Release.Namespace }} \
    +      -l component.autoscaling={{ .Release.Name }} \
    +      --wait || true
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    Why: Using multi-line YAML blocks for command arguments improves readability and maintainability. This suggestion enhances the clarity of the script, making it easier to read and modify in the future.

    6
    Best practice
    Use a constant for feature names to improve code maintainability

    Consider using a constant for the feature names 'DownloadBubble' and
    'DownloadBubbleV2' to improve maintainability and reduce the risk of typos.

    tests/SeleniumTests/init.py [132-133]

    +DOWNLOAD_FEATURES = 'DownloadBubble,DownloadBubbleV2'
     if not SELENIUM_ENABLE_MANAGED_DOWNLOADS:
    -    options.add_argument('disable-features=DownloadBubble,DownloadBubbleV2')
    +    options.add_argument(f'disable-features={DOWNLOAD_FEATURES}')
    • Apply this suggestion
    Suggestion importance[1-10]: 5

    Why: Using a constant for feature names improves maintainability and reduces the risk of typos. However, the impact is moderate as it primarily enhances code readability and consistency.

    5

    💡 Need additional feedback ? start a PR chat

    @VietND96 VietND96 force-pushed the trunk branch 2 times, most recently from 3177691 to 23ca623 Compare October 9, 2024 09:00
    Signed-off-by: Viet Nguyen Duc <nguyenducviet4496@gmail.com>
    @VietND96 VietND96 merged commit ea7b913 into SeleniumHQ:trunk Oct 10, 2024
    30 of 35 checks passed
    @VietND96
    Copy link
    Member Author

    Now, few GraphQL data is scraped to Prometheus. @Doofus100500, do you think a standard Grafana dashboard can be created (export as json, put in chart resources) and imported while installing chart?

    @Doofus100500
    Copy link
    Contributor

    @VietND96 I think everything should work out. During the Grafana deployment, you need to set sidecar.dashboards.enabled to true and add a ConfigMap with json to your chart with label: grafana_dashboard
    https://github.com/grafana/helm-charts/blob/main/charts/grafana/values.yaml#L970

    @VietND96
    Copy link
    Member Author

    Yes, I see. However, I am referring on what chart items, metrics we will visualize on a standard Dashboard that we will deliver.

    @Doofus100500
    Copy link
    Contributor

    Then I’ll just show mine.
    image
    image
    image
    image
    And the last one is about queue

    @yonigolob1
    Copy link

    @Doofus100500 would you mind sharing the queries as well, please?

    @Doofus100500
    Copy link
    Contributor

    @yonigolob1

    $serverInfoRequest = 'query { 
        grid { 
            uri, nodeCount, maxSession, sessionCount, version, sessionQueueSize 
        } 
    }'
    
    $sessionsInfoRequest = 'query {
        sessionsInfo {
            sessions {
                id,
                capabilities,
                startTime,
                uri,
                nodeId,
                nodeUri,
                sessionDurationMillis,
                slot { id, stereotype, lastStarted }
            }
        }
    }'
    
    $sessionsInQueueInfoRequest = 'query {
        sessionsInfo {
            sessionQueueRequests
        }
    }'
    
    $nodesInfoRequest = 'query {
        nodesInfo {
            nodes {
                id,
                uri,
                status,
                maxSession,
                slotCount,
                sessions {
                    id,
                    capabilities,
                    startTime,
                    uri,
                    nodeId,
                    nodeUri,
                    sessionDurationMillis,
                    slot {
                        id,
                        stereotype,
                        lastStarted
                    }
                },
                sessionCount,
                stereotypes,
                version,
                osInfo {
                    arch,
                    name,
                    version
                }
            }
        }
    }'
    

    @yonigolob1
    Copy link

    @Doofus100500 sorry my bad. I meant to the promql queries in the attached screenshots

    @Doofus100500
    Copy link
    Contributor

    Doofus100500 commented Nov 28, 2024

    We write metrics to Graphite

    @yonigolob1
    Copy link

    We write metrics to Graphite

    Gotcha. Thanks anyway

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

    Successfully merging this pull request may close these issues.

    3 participants