Skip to content

[1.15] cherrypick grpc keepalives (#1382)#1383

Merged
cicoyle merged 1 commit intodapr:release-1.15from
cicoyle:cherrypick-keepalives
May 21, 2025
Merged

[1.15] cherrypick grpc keepalives (#1382)#1383
cicoyle merged 1 commit intodapr:release-1.15from
cicoyle:cherrypick-keepalives

Conversation

@cicoyle
Copy link
Contributor

@cicoyle cicoyle commented May 21, 2025

* chore: Add grpc keepalives

Signed-off-by: Javier Aliaga <javier@diagrid.io>

* chore: Make grpc keepalive configurable

Signed-off-by: Javier Aliaga <javier@diagrid.io>

* chore: Fix review comments

Signed-off-by: Javier Aliaga <javier@diagrid.io>

* chore: Missing keepalive config for GRPC TLS INSECURE

Signed-off-by: Javier Aliaga <javier@diagrid.io>

* chore: Add test

Signed-off-by: Javier Aliaga <javier@diagrid.io>

* fix: Comment typo

Signed-off-by: Javier Aliaga <javier@diagrid.io>

---------

Signed-off-by: Javier Aliaga <javier@diagrid.io>
@cicoyle cicoyle requested review from a team as code owners May 21, 2025 14:55
Copy link
Contributor

@artur-ciocanu artur-ciocanu left a comment

Choose a reason for hiding this comment

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

@cicoyle as far as I understand this is a cherry pick, however the Java properties are a little bit inconsistent with other Java property names.

Here we are using a mix of dot notation and camel case, while for other GRPC properties we use dot notation like:

  • dapr.grpc.tls.insecure
  • dapr.grpc.tls.ca.path

Ideally we should have something like:

  • dapr.grpc.keepalive.enabled
  • dapr.grpc.keepalive.time.seconds
  • dapr.grpc.keepalive.timeout.seconds
  • etc

This way we have a nice name spacing approach to properties.

We should try to be consistent, it helps with documentation and long term maintenance.

CC: @javier-aliaga

@cicoyle
Copy link
Contributor Author

cicoyle commented May 21, 2025

@cicoyle as far as I understand this is a cherry pick, however the Java properties are a little bit inconsistent with other Java property names.

Here we are using a mix of dot notation and camel case, while for other GRPC properties we use dot notation like:

  • dapr.grpc.tls.insecure
  • dapr.grpc.tls.ca.path

We should try to be consistent, it helps with documentation and long term maintenance.

I'll open follow up PRs to address the inconsistency there, good catch 👍🏻 since you approved this, Ill proceed with this since its a cherrypick then open 2 followup PRs to fix it and add docs👍🏻

@cicoyle cicoyle merged commit 32f38d2 into dapr:release-1.15 May 21, 2025
11 of 12 checks passed
@codecov
Copy link

codecov bot commented May 21, 2025

Codecov Report

Attention: Patch coverage is 85.00000% with 6 lines in your changes missing coverage. Please review.

Please upload report for BASE (release-1.15@9f7c659). Learn more about missing BASE report.

Files with missing lines Patch % Lines
sdk/src/main/java/io/dapr/utils/NetworkUtils.java 78.57% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@               Coverage Diff               @@
##             release-1.15    #1383   +/-   ##
===============================================
  Coverage                ?   77.58%           
  Complexity              ?     1777           
===============================================
  Files                   ?      205           
  Lines                   ?     5474           
  Branches                ?      600           
===============================================
  Hits                    ?     4247           
  Misses                  ?      907           
  Partials                ?      320           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

siri-varma pushed a commit to siri-varma/java-sdk that referenced this pull request Aug 12, 2025
* chore: Add grpc keepalives

* chore: Make grpc keepalive configurable

* chore: Fix review comments

* chore: Missing keepalive config for GRPC TLS INSECURE

* chore: Add test

* fix: Comment typo

---------

Signed-off-by: Javier Aliaga <javier@diagrid.io>
Co-authored-by: Javier Aliaga <javier@diagrid.io>
Signed-off-by: siri-varma <siri.varma@outlook.com>
siri-varma pushed a commit to siri-varma/java-sdk that referenced this pull request Aug 12, 2025
* chore: Add grpc keepalives

* chore: Make grpc keepalive configurable

* chore: Fix review comments

* chore: Missing keepalive config for GRPC TLS INSECURE

* chore: Add test

* fix: Comment typo

---------

Signed-off-by: Javier Aliaga <javier@diagrid.io>
Co-authored-by: Javier Aliaga <javier@diagrid.io>
Signed-off-by: siri-varma <siri.varma@outlook.com>
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