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

Introduce HTTP OTLP exporter #35156

Merged
merged 1 commit into from
Aug 2, 2023
Merged

Introduce HTTP OTLP exporter #35156

merged 1 commit into from
Aug 2, 2023

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Aug 2, 2023

The integration utilizes the HttpSender
interface introduced in OTel 1.28
and is based on the Vert.x HTTP Client

Closes: #21535

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 2, 2023

/cc @brunobat (opentelemetry), @radcortez (opentelemetry)

@geoand geoand requested review from radcortez and brunobat August 2, 2023 06:16
@geoand
Copy link
Contributor Author

geoand commented Aug 2, 2023

@yrodiere I am seeing:

Error: Class initialization of org.h2.fulltext.FullTextLucene failed. This error is reported at image build time because class org.h2.fulltext.FullTextLucene is registered for linking at image build time by command line Use the option --initialize-at-run-time=org.h2.fulltext.FullTextLucene to explicitly request delayed initialization of this class.
Original exception that caused the problem: java.lang.NoClassDefFoundError: org/apache/lucene/index/IndexFormatTooOldException

in the main integration test and it should be completely unrelated to this PR (or at least I don't see how it could be related)

@yrodiere
Copy link
Member

yrodiere commented Aug 2, 2023

I'm back from two weeks out of office, so take this with a grain of salt, but all I can say is that I didn't see any change related to H2 recently, nor did I ever see any use of Lucene in Quarkus Core (let alone H2).

I suppose your changes might be introducing code that makes GraalVM think that FullTextLucene is reachable, even if it's not.

But be aware there is (used to be?) a bug in GraalVM in which the firsts static init error was ignored and GraalVM still proceeds and generates tons of seemingly nonsensical errors as a result. See oracle/graal#6368
EDIT: I originally reported the problem there, and I think the issue description includes instructions on how to debug this stuff.

I can have a look at some point, but I hope it's not too urgent as I'm still catching up with emails and (lots of) PRs that I need to review to get others unstuck.

EDIT: FWIW there's also this entry in H2's reflect-config.json, but personally I'd rather bet on the GraalVM bug.

@yrodiere
Copy link
Member

yrodiere commented Aug 2, 2023

Ah wait, I'm seeing this elsewhere, so it's not limited to this PR.

I guess I know what I'll do with my day :)

@yrodiere
Copy link
Member

yrodiere commented Aug 2, 2023

The caused seems to be there: #34615 (comment)

@geoand
Copy link
Contributor Author

geoand commented Aug 2, 2023

Thanks for the super quick investigation!

The integration utilizes the HttpSender
interface introduced in OTel 1.28
and is based on the Vert.x HTTP Client
Copy link
Contributor

@brunobat brunobat 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 to me. Thanks @geoand !

@geoand
Copy link
Contributor Author

geoand commented Aug 2, 2023

🙏🏼

@quarkus-bot

This comment has been minimized.

@geoand geoand merged commit fc53936 into quarkusio:main Aug 2, 2023
@quarkus-bot quarkus-bot bot added this to the 3.3 - main milestone Aug 2, 2023
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Aug 2, 2023
@geoand geoand deleted the #21535 branch August 3, 2023 07:34
@rsvoboda
Copy link
Member

rsvoboda commented Sep 4, 2023

@geoand Could you pls open PR for docs?

@geoand
Copy link
Contributor Author

geoand commented Sep 4, 2023

@rsvoboda not sure what you are asking for

@rsvoboda
Copy link
Member

rsvoboda commented Sep 4, 2023

This is new feature/enhancement, bu this PR didn't add anything into docs/guides.
Could you extend otel related guide(s) with information about OTLP/HTTP option?

@geoand
Copy link
Contributor Author

geoand commented Sep 4, 2023

That's probably best done by @brunobat who has all the OpenTelemetry context

@brunobat
Copy link
Contributor

brunobat commented Sep 4, 2023

I already did here:
https://quarkus.io/guides/opentelemetry#default
Please let me know if additional clarification is needed @rsvoboda

PS: PR #35465

@rsvoboda
Copy link
Member

rsvoboda commented Sep 4, 2023

Thanks Bruno. We have a note, should be good for now. It can be extended if people need more details.

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.

Provide OpenTelemetry OTLP/HTTP exporter
5 participants