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

Add OTEL smoke test #268

Merged
merged 13 commits into from
Feb 18, 2021
Merged

Add OTEL smoke test #268

merged 13 commits into from
Feb 18, 2021

Conversation

samarth-gupta-traceable
Copy link
Contributor

@samarth-gupta-traceable samarth-gupta-traceable commented Feb 17, 2021

Updates #217

Description

Please include a summary of the change, motivation and context.

Testing

Please describe the tests that you ran to verify your changes. Please summarize what did you test and what needs to be tested e.g. deployed and tested helm chart locally.

ran ./gradlew build locally

Documentation

Make sure that you have documented corresponding changes in this repository or hypertrace docs repo if required.

Copy link
Member

@pavolloffay pavolloffay left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the PR 👍

We should try to remove those two gradle scripts and reduce CI run time

Makefile Outdated
@@ -7,11 +7,11 @@ assemble:

.PHONY: build
build:
./gradlew build -x :smoke-tests:test --stacktrace
./gradlew build -x :smoke-tests:test --debug --stacktrace
Copy link
Member

Choose a reason for hiding this comment

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

is this needed? I don't like to run debug locally. As below in the smoke-test target

Copy link
Contributor Author

Choose a reason for hiding this comment

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

have removed the debug option

@@ -0,0 +1,97 @@
def groovyVer = "2.5.11"
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this file and the java.gradle?

in OTEL repo these are reused for other modules. since we have only smoke tests module we could copy there only what is needed. For instance we define versions in our root build script.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pavolloffay have removed dependencies.gradle but not java.gradle.

@pavolloffay pavolloffay changed the title Issue 217 otel smoke test Add OTEL smoke test Feb 18, 2021
@pavolloffay
Copy link
Member

@samarth-gupta-traceable the next step will be to modify the apps to include /echo endpoint and add a test with posts JSON (and receives it).

@samarth-gupta-traceable samarth-gupta-traceable merged commit dad264d into main Feb 18, 2021
@samarth-gupta-traceable samarth-gupta-traceable deleted the issue_217_otel_smoke_test branch February 18, 2021 12:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants