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

Call Prometheus, Loki and Jaeger via Grafana API #13973

Merged
merged 45 commits into from Apr 25, 2022

Conversation

dennis-ge
Copy link
Contributor

@dennis-ge dennis-ge commented Apr 13, 2022

Description

Changes proposed in this pull request:

  • Calling prometheus, jaeger, and loki via the Grafana datasource API (see proxyGrafanaDashboard).
    • By doing this, we get rid of kubectl forward (see kubectlportForward), and execute the integration tests more from a user perspective.
    • The initial idea was to call the services via the apiserver proxy. This worked for prometheus but didn't work for loki and jaeger as istio authorizationpolicies and peerauthorization resources restricted the access to these services.
  • As a consequence, Grafana needs to be exposed before the first call to any datasource (prometheus, jaeger, and loki) is triggered. That's why the monitoring tests are now executed first in the standard test suite. The Grafana proxy is then reset at the end of the test.
  • Fixing type coercion
  • Use the new logging functions (info, error) in the observability tests
  • Extract Jaeger client calls to tracing directory

Pipelines:

Related issue(s)
Fixes #13375

@kyma-bot kyma-bot added the area/tests Issues or PRs related to tests label Apr 13, 2022
@dennis-ge
Copy link
Contributor Author

/hold

@kyma-bot kyma-bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 13, 2022
@netlify
Copy link

netlify bot commented Apr 13, 2022

🥰 Documentation preview ready! 🥰

Name Link
🔨 Latest commit 5c47024
🔍 Latest deploy log https://app.netlify.com/sites/kyma-project-docs-preview/deploys/62612f2b657cb90009aa8fd6
😎 Deploy Preview https://deploy-preview-13973--kyma-project-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@dennis-ge dennis-ge marked this pull request as draft April 14, 2022 12:29
@kyma-bot kyma-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 14, 2022
@kyma-bot kyma-bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 14, 2022
@dennis-ge
Copy link
Contributor Author

/test pre-main-kyma-integration-k3d-central-app-connectivity

@dennis-ge
Copy link
Contributor Author

/test pre-main-kyma-gardener-gcp-eventing-upgrade
/test post-main-kyma-gardener-azure-alpha-eval

@kyma-bot kyma-bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Apr 22, 2022
@dennis-ge
Copy link
Contributor Author

/retest

@kyma-bot kyma-bot added the lgtm Looks good to me! label Apr 22, 2022
@dennis-ge
Copy link
Contributor Author

/retest

@kyma-bot kyma-bot removed the lgtm Looks good to me! label Apr 25, 2022
@dennis-ge
Copy link
Contributor Author

/retest

@dennis-ge
Copy link
Contributor Author

/test pre-main-kyma-integration-k3d-central-app-connectivity
/test pre-main-kyma-skr-eventing
/test pre-main-kyma-gardener-gcp-eventing

@dennis-ge
Copy link
Contributor Author

/retest

console.log('Checking redirect for kyma docs');
return await retryUrl(url, redirectURL, ignoreSSL, 403);
async function setGrafanaProxy() {
if (getEnvOrDefault('KYMA_MAJOR_VERSION', '2') === '2') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we still need it? I thought all Kyma 1.* remnants are gone

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would keep it for now as it is still used in other places in the code and remove them all in one follow-up PR.

tests/fast-integration/monitoring/grafana.js Show resolved Hide resolved
await updateProxyDeployment('--reverse-proxy=true', '--trusted-ip=0.0.0.0/0');
info('Checking grafana redirect to grafana URL');
const res = await checkGrafanaRedirect('https://grafana.', 200);
assert.isTrue(res, 'Grafana redirect to grafana landing page does not work!');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should assert inside helper functions? Maybe throw an exception?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also assert in other helper functions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still think it hinders test glanceability, but let's keep it for now

tests/fast-integration/monitoring/grafana.js Outdated Show resolved Hide resolved
tests/fast-integration/eventing-test/eventing-test.js Outdated Show resolved Hide resolved
tests/fast-integration/package-lock.json Outdated Show resolved Hide resolved
@skhalash
Copy link
Contributor

/lgtm

@kyma-bot kyma-bot added the lgtm Looks good to me! label Apr 25, 2022
@skhalash
Copy link
Contributor

/approve

@kyma-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: skhalash, VladislavPaskar

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kyma-bot kyma-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 25, 2022
@skhalash
Copy link
Contributor

/unhold

@kyma-bot kyma-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 25, 2022
@kyma-bot
Copy link
Contributor

kyma-bot commented Apr 25, 2022

@dennis-ge: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
post-main-kyma-gardener-azure-alpha-eval 13b9bb6 link unknown ``
pre-main-kyma-integration-k3d-central-app-connectivity-compass 9d5f5b3 link false /test pre-main-kyma-integration-k3d-central-app-connectivity-compass
pre-main-kyma-skr-eventing 9d5f5b3 link false /test pre-main-kyma-skr-eventing

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@dennis-ge
Copy link
Contributor Author

/retest-required

@kyma-bot kyma-bot merged commit 138bb64 into kyma-project:main Apr 25, 2022
koala7659 pushed a commit to koala7659/kyma that referenced this pull request May 5, 2022
* add callServiceViaProxy, remove pf references, fix unexpected type coercion

* add async await

* add log statement

* add return

* call jaeger and loki via grafana

* switch to new info and error functions

* add await

* first execute grafana tests

* don't reset proxy

* comment out reset proxy

* add async await

* improve logging

* get dashboard id before querying grafana

* remove dedicated httpsAgent

* fix jaeger datasource id

* remove opts args

* outsource logic into proxyGrafanaDatasource

* call prometheus via grafana

* lint

* cleanup

* fix upgrade test

* update eventing tests

* refactor grafana tests

* remove port forward from telemetry test

* add async await

* telemetry test before and after

* update telemetry test

* use mocha it

* add setGrafanaProxy method

* remove unused var

* update mocha tests

* adjust ordering of tests

* remove resetting proxy

* small refactoring of grafana.js

* fix lint

* change it to after

* change to mocha describe

* uncomment code

* small refactoring

* revert unintended changes

* fix skr

* apply suggestions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/eventing Issues or PRs related to eventing area/tests Issues or PRs related to tests lgtm Looks good to me! size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix socket hangup during test execution
4 participants