Skip to content

dynamic_modules: add generic accessors for access logger#43647

Merged
agrawroh merged 8 commits into
envoyproxy:mainfrom
agrawroh:feat-hal-2
Mar 25, 2026
Merged

dynamic_modules: add generic accessors for access logger#43647
agrawroh merged 8 commits into
envoyproxy:mainfrom
agrawroh:feat-hal-2

Conversation

@agrawroh
Copy link
Copy Markdown
Member

Description


Commit Message: dynamic_modules: add generic accessors for access logger
Additional Description:
Risk Level: Low
Testing: CI
Docs Changes: Added
Release Notes: N/A

@repokitteh-read-only
Copy link
Copy Markdown

As a reminder, PRs marked as draft will not be automatically assigned reviewers,
or be handled by maintainer-oncall triage.

Please mark your PR as ready when you want it to be reviewed!

🐱

Caused by: #43647 was opened by agrawroh.

see: more, trace.

@agrawroh agrawroh force-pushed the feat-hal-2 branch 2 times, most recently from dc37798 to 13883db Compare February 25, 2026 22:47
@agrawroh agrawroh marked this pull request as ready for review February 25, 2026 23:06
Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
mathetake
mathetake previously approved these changes Feb 26, 2026
Copy link
Copy Markdown
Member

@mathetake mathetake left a comment

Choose a reason for hiding this comment

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

Thank you! This is much clearer and more consistent

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
mathetake
mathetake previously approved these changes Feb 26, 2026
Copy link
Copy Markdown
Member

@wbpcode wbpcode left a comment

Choose a reason for hiding this comment

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

Thanks for this great contribution.

I think this is a break change to ABI because the previous AIB have been released in 1.37? Could we keep previous code (ABI and ABI implementation) but mark them as deprecated and remove them after the it's out of supported?

@wbpcode
Copy link
Copy Markdown
Member

wbpcode commented Feb 26, 2026

ping me we we get agreement about the break change here is acceptable here.

@mathetake
Copy link
Copy Markdown
Member

yeah let's do the deprecation stuff

@agrawroh
Copy link
Copy Markdown
Member Author

Yeah, I will depreciate them instead of removing. Good callout!

@paul-r-gall
Copy link
Copy Markdown
Contributor

@agrawroh ping on deprecation rather than removal

@paul-r-gall
Copy link
Copy Markdown
Contributor

/wait

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
* @param logger_envoy_ptr is the pointer to the log context.
* @return true if this is a health check, false otherwise.
*/
bool envoy_dynamic_module_callback_access_logger_is_health_check(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

what is this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yeah, we are not deprecating it and it's already part of Access Logger. I just need to remove it from here.

Comment thread source/extensions/dynamic_modules/dynamic_modules.bzl Outdated
Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
Copy link
Copy Markdown
Member

@wbpcode wbpcode left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

Comment on lines -6420 to +6448
bool envoy_dynamic_module_callback_access_logger_get_upstream_tls_version(
bool envoy_dynamic_module_callback_access_logger_get_upstream_tls_cipher(
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could we ensure this ABI is not released in 1.37?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I guess yeah

@agrawroh agrawroh merged commit 6874e47 into envoyproxy:main Mar 25, 2026
29 checks passed
TAOXUY pushed a commit to TAOXUY/envoy that referenced this pull request Apr 1, 2026
…43647)

## Description

---

**Commit Message:** dynamic_modules: add generic accessors for access
logger
**Additional Description:**
**Risk Level:** Low
**Testing:** CI
**Docs Changes:** Added
**Release Notes:** N/A

---------

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
Signed-off-by: Xuyang Tao <taoxuy@google.com>
citrus7 pushed a commit to citrus7/envoy that referenced this pull request Apr 1, 2026
…43647)

## Description

---

**Commit Message:** dynamic_modules: add generic accessors for access
logger
**Additional Description:**
**Risk Level:** Low
**Testing:** CI
**Docs Changes:** Added
**Release Notes:** N/A

---------

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
Signed-off-by: Jonathan Wu <jtwu@google.com>
nshipilov pushed a commit to nshipilov/envoy that referenced this pull request Apr 13, 2026
…43647)

## Description

---

**Commit Message:** dynamic_modules: add generic accessors for access
logger
**Additional Description:**
**Risk Level:** Low
**Testing:** CI
**Docs Changes:** Added
**Release Notes:** N/A

---------

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
Signed-off-by: Nick Shipilov <nick.shipilov.n@gmail.com>
krinkinmu pushed a commit to grnmeira/envoy that referenced this pull request Apr 20, 2026
…43647)

## Description

---

**Commit Message:** dynamic_modules: add generic accessors for access
logger
**Additional Description:**
**Risk Level:** Low
**Testing:** CI
**Docs Changes:** Added
**Release Notes:** N/A

---------

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.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.

4 participants