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.

Sign up for GitHub

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

Jump to bottom

create metricexporterfactory by default #1098

Merged
merged 5 commits into from
Aug 27, 2020
Merged

create metricexporterfactory by default #1098

merged 5 commits into from
Aug 27, 2020

Conversation

wyTrivail
Copy link
Contributor

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 25, 2020 •
edited
Loading

CLA Check
The committers are authorized under a signed CLA.

@wyTrivail
Copy link
Contributor Author

just signed CLA

@wyTrivail wyTrivail mentioned this pull request Aug 25, 2020
Closed
@wyTrivail
Copy link
Contributor Author

got an error below, should i modify the version number somewhere?

Caused by: org.gradle.internal.resolve.ModuleVersionNotFoundException: Could not find io.opentelemetry:opentelemetry-sdk:0.8.0-20200811.154243-20.
Searched in the following locations:
  - file:/home/circleci/.m2/repository/io/opentelemetry/opentelemetry-sdk/0.8.0-SNAPSHOT/opentelemetry-sdk-0.8.0-20200811.154243-20.pom
  - https://jcenter.bintray.com/io/opentelemetry/opentelemetry-sdk/0.8.0-SNAPSHOT/opentelemetry-sdk-0.8.0-20200811.154243-20.pom
  - https://repo.maven.apache.org/maven2/io/opentelemetry/opentelemetry-sdk/0.8.0-SNAPSHOT/opentelemetry-sdk-0.8.0-20200811.154243-20.pom
  - https://repo.typesafe.com/typesafe/releases/io/opentelemetry/opentelemetry-sdk/0.8.0-SNAPSHOT/opentelemetry-sdk-0.8.0-20200811.154243-20.pom
  - https://oss.jfrog.org/artifactory/oss-snapshot-local/io/opentelemetry/opentelemetry-sdk/0.8.0-SNAPSHOT/opentelemetry-sdk-0.8.0-20200811.154243-20.pom
Required by:
    project :opentelemetry-sdk-shaded-for-testing

trask
trask reviewed Aug 26, 2020
View reviewed changes
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

Thanks @wyTrivail!

I'm not sure why the build is failing, it doesn't seem related to your PR, I will look into it.

javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/TracerInstaller.java Outdated
DefaultExporterConfig config = new DefaultExporterConfig("exporter");
installExporter(metricExporterFactory, config);
} else {
log.warn("No {} metric exporter found", exporterName);
Copy link
Member

@trask trask Aug 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

will this log warning for jaeger/zipkin users?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point! Yes it will. I can remove this warning to reduce the confusion to jaeger/zipkin users if you don't mind.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good, maybe debug then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, just updated.

@trask
Copy link
Member

trask commented Aug 26, 2020

hey @wyTrivail, CI is failing due to #1102, we'll keep you posted once we get this resolved in master.

@wyTrivail
only print debug level message when there's no metric exporter factor…
430e3c5 
…y to reduce confusion on jaeger/zipkin users
trask
trask approved these changes Aug 26, 2020
View reviewed changes
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

Hey @wyTrivail, our build issues are fixed in master now, can you merge master into your branch?

iNikem and others added 3 commits August 26, 2020 13:58
@iNikem @wyTrivail
Update to latest otel-java snapshot (open-telemetry#1057)
e28151c 
* Update to latest otel-java snapshot

* Upgrade to working otel-java snapshot

* Update to latest otel-java snapshot

* Update to latest otel-java snapshot
@trask @wyTrivail
Remove executor queue offer check (open-telemetry#1090)
d791baf
@wyTrivail
Merge branch 'master' of github.com:open-telemetry/opentelemetry-java…
ec0cdd8 
…-instrumentation into master
@wyTrivail
Copy link
Contributor Author

Hey @wyTrivail, our build issues are fixed in master now, can you merge master into your branch?

sure, just updated.

@trask trask merged commit cfd1119 into open-telemetry:master Aug 27, 2020
@trask
Copy link
Member

trask commented Aug 27, 2020

Thanks @wyTrivail!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@trask trask trask approved these changes

@iNikem iNikem iNikem approved these changes

@anuraaga anuraaga Awaiting requested review from anuraaga

@jkwatson jkwatson Awaiting requested review from jkwatson

@tylerbenson tylerbenson Awaiting requested review from tylerbenson

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@wyTrivail @trask @iNikem