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 kamon-pekko instrumentation #1264

Merged
merged 5 commits into from Aug 9, 2023
Merged

Add kamon-pekko instrumentation #1264

merged 5 commits into from Aug 9, 2023

Conversation

DieBauer
Copy link
Contributor

@DieBauer DieBauer commented Feb 28, 2023

Since Akka changed its license to BSL, an Apache fork of Akka was made: Apache Pekko (and Apache Pekko Http).

An initial release is released, and the major change is that the namespaces changed from akka. to org.apache.pekko..

This PR adds the first module that copy the behavior of kamon-akka.

The changes can be summed up as follows:

  • Change the package imports to org.apache.pekko.
  • Change configuration keys from akka to pekko.
  • Remove the Common / Akka25 / Akka26 complexity from kamon-pekko. Baseline is the last Apache 2.0 license release of Akka: 2.6.20.

All tests are green.

Is this something you want to accept in Kamon?

"correctly time entity transfer timings" in {
val target = s"$protocol://$interface:$port/$stream"

try {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting finding here, the akka-stream dependency is 'hard-coded' to 2.5.32 in the kamon-akka-http module. When this dependency is updated to 2.6.20 the test fails in the same way. the first server (HTTP) gets an exception. However the metrics etc are already recorded in Kamon, hence a green test when the exception handling is a no-op.

java.net.SocketException: Connection reset
  | at java.net.SocketInputStream.read(SocketInputStream.java:210)
	at java.net.SocketInputStream.read(SocketInputStream.java:141)
	at okio.InputStreamSource.read(JvmOkio.kt:94)
	at okio.AsyncTimeout$source$1.read(AsyncTimeout.kt:125)
	at okio.RealBufferedSource.indexOf(RealBufferedSource.kt:427)
	at okio.RealBufferedSource.readUtf8LineStrict(RealBufferedSource.kt:320)
	at okhttp3.internal.http1.HeadersReader.readLine(HeadersReader.kt:29)
	at okhttp3.internal.http1.Http1ExchangeCodec.readResponseHeaders(Http1ExchangeCodec.kt:178)
	at okhttp3.internal.connection.Exchange.readResponseHeaders(Exchange.kt:106)
	at okhttp3.internal.http.CallServerInterceptor.intercept(CallServerInterceptor.kt:79)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
	at okhttp3.internal.connection.ConnectInterceptor.intercept(ConnectInterceptor.kt:34)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
	at okhttp3.internal.cache.CacheInterceptor.intercept(CacheInterceptor.kt:95)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
	at okhttp3.internal.http.BridgeInterceptor.intercept(BridgeInterceptor.kt:83)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
	at okhttp3.internal.http.RetryAndFollowUpInterceptor.intercept(RetryAndFollowUpInterceptor.kt:76)
	at okhttp3.internal.http.RealInterceptorChain.proceed(RealInterceptorChain.kt:109)
	at okhttp3.internal.connection.RealCall.getResponseWithInterceptorChain$okhttp(RealCall.kt:201)
	at okhttp3.internal.connection.RealCall.execute(RealCall.kt:154)
	at kamon.pekko.http.PekkoHttpServerTracingSpec.$anonfun$testSuite$27(PekkoHttpServerTracingSpec.scala:233)

@DieBauer DieBauer marked this pull request as ready for review March 3, 2023 10:11
@DieBauer
Copy link
Contributor Author

@ivantopo any interest in taking this forward? Or does Kamon not want to support Pekko?

@ivantopo
Copy link
Contributor

Hey @DieBauer 👋

Yeap, I'm totally up for this! I see that there is a 1.0.0-rc2 tag on the Pekko repository. Is the 1.0.0 release close? It would be great to have Kamon support together with the 1.0.0 release!

@DieBauer
Copy link
Contributor Author

Great, thanks for letting me know! Yes 1.0.0 of Pekko is in the making, I don't have a date though. Pekko HTTP should follow suit. I'll update once they are on 1.0.0!

@DieBauer DieBauer changed the title Add kamon-pekko and kamon-pekko-http Add kamon-pekko Jul 15, 2023
@DieBauer
Copy link
Contributor Author

I have updated the branch to only include the 1.0.0 release of Apache Pekko. I will create a PR for pekko-http (still no 1.0.0 release). so we could go ahead and cut a Kamon release for only Pekko?

@DieBauer DieBauer mentioned this pull request Jul 15, 2023
@DieBauer DieBauer changed the title Add kamon-pekko Add kamon-pekko instrumentation Jul 27, 2023
@DieBauer
Copy link
Contributor Author

@ivantopo could you tell me your plan (priorities) for getting this feature / module in Kamon?

@ivantopo
Copy link
Contributor

ivantopo commented Aug 1, 2023

@DieBauer to be honest, this is such a big PR that I don't think I can review it fully and successfully. I usually test Akka stuff on test applications I have around but I don't have anything converted to Pekko yet (and all of our prod stuff right now actually using Armeria 🤷)

I'm thinking that the best move forward would be to merge the PR, publish a release, and add you as a maintainer in Kamon so that maybe if some minor fixes need to be merged/released you can do that independently of whether I'm available or not :D. What do you think?

@mebigfatguy
Copy link

I am very interested in both kamon-pekko and kamon-pekko-http as well, if you would be willing to pull this into branches i would look forward to testing it on my existing stuff that i've just switched to pekko.

@ivantopo ivantopo merged commit 2570a11 into kamon-io:master Aug 9, 2023
1 check passed
@ivantopo
Copy link
Contributor

ivantopo commented Aug 9, 2023

Thank you very much @DieBauer and everyone who helped make this happen 🎉

I'll take a look at the Pekko HTTP now and see if we can merge and release everything together!

hughsimpson pushed a commit to hughsimpson/Kamon that referenced this pull request Aug 22, 2023
* Add kamon-pekko module for Apache Pekko support

Replace with advice should be in correct package

* Use released 1.0.0 for Apache Pekko

* Update sbt to 1.9.2

* Update Pekko to 1.0.1

https://pekko.apache.org/docs/pekko/1.0/release-notes/index.html#1-0-1

* Change scaladoc references from Akka to Pekko
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.

None yet

4 participants