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 go example #14

Merged
merged 6 commits into from
Feb 5, 2024
Merged

add go example #14

merged 6 commits into from
Feb 5, 2024

Conversation

zeitlinger
Copy link
Member

No description provided.

@@ -0,0 +1,16 @@
#!/bin/bash

set -euo pipefail
Copy link
Member Author

Choose a reason for hiding this comment

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

file diff doesn't show, this was the old version:

#!/bin/bash

cd example-app
if [[ ! -f ./target/example-app.jar ]] ; then
    ./mvnw clean package
fi
if [[ ! -f ./opentelemetry-javaagent.jar ]] ; then
    curl -sOL https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/download/v2.0.0/opentelemetry-javaagent.jar
fi
export OTEL_EXPORTER_OTLP_PROTOCOL=grpc
export OTEL_SEMCONV_STABILITY_OPT_IN=http
export OTEL_RESOURCE_ATTRIBUTES="service.name=example-app,service.instance.id=localhost:8080"
java -Dotel.metric.export.interval=500 -Dotel.bsp.schedule.delay=500 -javaagent:opentelemetry-javaagent.jar -jar ./target/example-app.jar

Copy link
Member Author

Choose a reason for hiding this comment

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

in the new version

  • removed export OTEL_SEMCONV_STABILITY_OPT_IN=http that is no longer needed
  • don't reuse old downloaded jar if the version doesn't match

@zeitlinger zeitlinger marked this pull request as ready for review February 2, 2024 17:11
@zeitlinger zeitlinger requested a review from fstab February 2, 2024 17:11
@zeitlinger zeitlinger self-assigned this Feb 2, 2024
Copy link
Contributor

@grcevski grcevski left a comment

Choose a reason for hiding this comment

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

LGTM! Great work!

I just have one comment/question, do you think it's OK to change the default port for the Go application so it doesn't conflict with what Java uses? That way we can run both at the same time...

Copy link
Member

@fstab fstab left a comment

Choose a reason for hiding this comment

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

LGTM too, thanks for adding a Go example!

Optional remark: We could add a comment in the OATS-related files with a link to https://github.com/grafana/oats in case people are wondering what these files are for.

@zeitlinger
Copy link
Member Author

I just have one comment/question, do you think it's OK to change the default port for the Go application so it doesn't conflict with what Java uses? That way we can run both at the same time...

@grcevski the traffic script goes to 8080 - should this script hit all applications?

@grcevski
Copy link
Contributor

grcevski commented Feb 5, 2024

I just have one comment/question, do you think it's OK to change the default port for the Go application so it doesn't conflict with what Java uses? That way we can run both at the same time...

@grcevski the traffic script goes to 8080 - should this script hit all applications?

Hm yeah, I am not sure what's best then. Maybe leave it as is, if it goes to both, then what if a user only boots Java, the script will hit something else or non existent port.

At least with duplicate port they will see it and adjust if they need to. LGTM with out my suggestion.

@zeitlinger
Copy link
Member Author

Hm yeah, I am not sure what's best then. Maybe leave it as is, if it goes to both, then what if a user only boots Java, the script will hit something else or non existent port.

At least with duplicate port they will see it and adjust if they need to. LGTM with out my suggestion.

I hit all ports - but ignore errors

@grcevski
Copy link
Contributor

grcevski commented Feb 5, 2024

Hm yeah, I am not sure what's best then. Maybe leave it as is, if it goes to both, then what if a user only boots Java, the script will hit something else or non existent port.
At least with duplicate port they will see it and adjust if they need to. LGTM with out my suggestion.

I hit all ports - but ignore errors

Great!

@zeitlinger zeitlinger merged commit 5e7bfc9 into main Feb 5, 2024
2 checks passed
@zeitlinger zeitlinger deleted the go-example branch February 5, 2024 17:25
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.

3 participants