Skip to content

xDS: add xDS config tracker extension point#23485

Merged
mattklein123 merged 43 commits intoenvoyproxy:mainfrom
botengyao:xds-metadata-extension-point
Dec 13, 2022
Merged

xDS: add xDS config tracker extension point#23485
mattklein123 merged 43 commits intoenvoyproxy:mainfrom
botengyao:xds-metadata-extension-point

Conversation

@botengyao
Copy link
Copy Markdown
Member

@botengyao botengyao commented Oct 13, 2022

Signed-off-by: Boteng Yao boteng@google.com

This extension added hooks to allow tracking xDS responses or resources in external components,
e.g., external tracer or monitor. It provides the process point when receive, ingest, or fail to process xDS resources and messages.

Commit Message:
Additional Description:
Risk Level: L
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Boteng Yao <boteng@google.com>
@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: #23485 was opened by botengyao.

see: more, trace.

@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @htuch
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #23485 was opened by botengyao.

see: more, trace.

Signed-off-by: Boteng Yao <boteng@google.com>
Signed-off-by: Boteng Yao <boteng@google.com>
@botengyao botengyao changed the title [WIP] xDS: add xDS tracer extension point [WIP] xDS: add xDS config tracer extension point Oct 14, 2022
@botengyao botengyao changed the title [WIP] xDS: add xDS config tracer extension point [WIP] xDS: add xDS config trace logger extension point Oct 14, 2022
Signed-off-by: Boteng Yao <boteng@google.com>
…ion-point

Signed-off-by: Boteng Yao <boteng@google.com>
Signed-off-by: Boteng Yao <boteng@google.com>
Signed-off-by: Boteng Yao <boteng@google.com>
Signed-off-by: Boteng Yao <boteng@google.com>
@botengyao botengyao marked this pull request as ready for review October 24, 2022 21:15
@botengyao botengyao changed the title [WIP] xDS: add xDS config trace logger extension point xDS: add xDS config trace logger extension point Oct 24, 2022
Signed-off-by: Boteng Yao <boteng@google.com>
…ion-point

Signed-off-by: Boteng Yao <boteng@google.com>
@botengyao
Copy link
Copy Markdown
Member Author

botengyao commented Oct 26, 2022

Hi @htuch, @adisuissa, sorry for the delay, can we kick off the discussion & review process for this PR? I am mainly adding this as a log point for some external library (e.g., internal crumbs usage). Thank you!

Signed-off-by: Boteng Yao <boteng@google.com>
Signed-off-by: Boteng Yao <boteng@google.com>
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa 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 working on this, very useful!
Left a few high-level design comments.

…ion-point

Signed-off-by: Boteng Yao <boteng@google.com>
Signed-off-by: Boteng Yao <boteng@google.com>
…ion-point

Signed-off-by: Boteng Yao <boteng@google.com>
…ion-point

Signed-off-by: Boteng Yao <boteng@google.com>
Signed-off-by: Boteng Yao <boteng@google.com>
@botengyao
Copy link
Copy Markdown
Member Author

/assign @envoyproxy/senior-maintainers

Can I get another pass for this PR? thanks!

@repokitteh-read-only
Copy link
Copy Markdown

@envoyproxy/senior-maintainers cannot be assigned to this issue.

🐱

Caused by: a #23485 (comment) was created by @botengyao.

see: more, trace.

@botengyao
Copy link
Copy Markdown
Member Author

/assign @mattklein123

Copy link
Copy Markdown
Contributor

@adisuissa adisuissa left a comment

Choose a reason for hiding this comment

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

Thanks, left a few minor comments.
/wait

Signed-off-by: Boteng Yao <boteng@google.com>
…ion-point

Signed-off-by: Boteng Yao <boteng@google.com>
Signed-off-by: Boteng Yao <boteng@google.com>
adisuissa
adisuissa previously approved these changes Dec 12, 2022
Copy link
Copy Markdown
Contributor

@adisuissa adisuissa 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!

@botengyao
Copy link
Copy Markdown
Member Author

Hi @mattklein123, @lizan, PTAL and can we get this merged? Thanks!

// e.g., external tracer or monitor. It provides the process point when receive, ingest, or fail to
// process xDS resources and messages.
// If a value is not specified, no XdsConfigTracker will be used.
// [#not-implemented-hide:]
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.

Why is this not-implemented-hide? Isn't it implemented? Also, can you please add a release note? Thank you.

/wait

Copy link
Copy Markdown
Member Author

@botengyao botengyao Dec 12, 2022

Choose a reason for hiding this comment

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

Thanks! This PR only adds an api extension interface right now, and there is no actual extension implementation. This enables the ability to extend this interface for different use cases. Should I add a release note only for this interface?

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.

The fact that it can be implemented means it's implemented, so I would remove the not-implemented-hide and still add a release note for it. It would be also good to :repo: link to the test extension so people can look at an example of the API, and I would also mention there are no in-repo extensions currently, etc.

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.

Thanks! Done, and waiting CI.

Signed-off-by: Boteng Yao <boteng@google.com>
…ion-point

Signed-off-by: Boteng Yao <boteng@google.com>
@botengyao
Copy link
Copy Markdown
Member Author

/retest

@repokitteh-read-only
Copy link
Copy Markdown

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #23485 (comment) was created by @botengyao.

see: more, trace.

Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks!

@mattklein123 mattklein123 enabled auto-merge (squash) December 13, 2022 15:10
@mattklein123 mattklein123 merged commit 2681bdd into envoyproxy:main Dec 13, 2022
jpsim added a commit that referenced this pull request Dec 14, 2022
…-tools

* origin/main: (59 commits)
  Make IsOkAndHolds matcher work with submatchers (#24498)
  ios: fix platform key value store (#24532)
  make ClusterInfo::traffic_stats_ a unique_ptr, so that later we can lazy-init it later. (#24406)
  quic: splitting into client and server (#24513)
  fixing coverage (#24529)
  ci: Add `examplesOnly` condition (#24465)
  ci: sonatype_nexus_upload.py: remove unused format argument (#24471)
  deps: Bump `build_bazel_rules_apple` -> 1.1.3 (#24527)
  deps: Bump `com_github_nghttp2_nghttp2` -> 1.51.0 (#24525)
  deps: Bump `rules_license` -> 0.0.4 (#24523)
  build(deps): bump sphinxcontrib-httpdomain from 1.8.0 to 1.8.1 in /mobile/docs (#24126)
  build(deps): bump github/codeql-action from 2.1.35 to 2.1.36 (#24473)
  build(deps): bump openpolicyagent/opa from 0.47.2-istio to 0.47.3-istio in /examples/ext_authz (#24514)
  build(deps): bump node from `80844b6` to `2770c78` in /examples/ext_authz/auth/http-service (#24515)
  build(deps): bump abseil-cpp to latest version (#24386)
  xDS: add xDS config tracker extension point (#23485)
  kafka: add shared consumer manager (#24494)
  coverage: Improve test coverage (#24355)
  deps: Bump `rules_python` -> 0.16.1 (#24344)
  ci: revert disable running the Objective-C integration app (#24478)" (#24496)
  ...

Signed-off-by: JP Simard <jp@jpsim.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.

6 participants