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.

Sign up for GitHub

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

Jump to bottom

ext_proc: observability mode part 3: logging and billing support #34960

Merged
merged 5 commits into from
Jul 3, 2024
Merged

ext_proc: observability mode part 3: logging and billing support #34960

merged 5 commits into from
Jul 3, 2024

Conversation

tyxia
Copy link
Member

@tyxia tyxia commented Jun 27, 2024 •
edited
Loading

There is no response in observability mode by design, so logging on onFinishProcessorCalls which is invoked on response won't work. Note, existing error/close case still work as normal

Solution: Leverage the encodeComplete function to write the logging/billing info. This is also before the consumers (extensions/filters behind) reading it.

Test: Existing Integration test frame with logging_test_filter as consumer

@repokitteh-read-only RepoKitteh - Read Only
Copy link

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: #34960 was opened by tyxia.

see: more, trace.

@tyxia
remove blank
9cf0cbb 
Signed-off-by: tyxia <tyxia@google.com>
@tyxia
remove TODO
7e61d8e 
Signed-off-by: tyxia <tyxia@google.com>
@tyxia
Copy link
Member Author

tyxia commented Jun 28, 2024

/retest

@tyxia tyxia marked this pull request as ready for review June 28, 2024 01:53
@tyxia
Copy link
Member Author

tyxia commented Jun 28, 2024

/assign @htuch @yanavlasov

@tyxia
add check
eef44f6 
Signed-off-by: tyxia <tyxia@google.com>
htuch
htuch reviewed Jul 3, 2024
View reviewed changes
Copy link
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

LGTM modulo nits.

@tyxia
change to lower case
82149e4 
Signed-off-by: tyxia <tyxia@google.com>
@tyxia
Copy link
Member Author

tyxia commented Jul 3, 2024

/retest

@htuch htuch merged commit 45c1b75 into envoyproxy:main Jul 3, 2024
50 of 51 checks passed
@tyxia tyxia deleted the ob_logging branch July 5, 2024 14:56
cainelli pushed a commit to cainelli/envoy that referenced this pull request Jul 5, 2024
@tyxia @cainelli
ext_proc: observability mode part 3: logging and billing support (env…
e828a7d 
…oyproxy#34960)

There is no response in observability mode by design, so logging on onFinishProcessorCalls which is invoked on response won't work. Note, existing error/close case still work as normal

Solution: Leverage the encodeComplete function to write the logging/billing info. This is also before the consumers (extensions/filters behind) reading it.

Test: Existing Integration test frame with logging_test_filter as consumer
Risk level: Low

Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: Fernando Cainelli <fernando.cainelli-external@getyourguide.com>
cainelli pushed a commit to cainelli/envoy that referenced this pull request Jul 5, 2024
@tyxia @cainelli
ext_proc: observability mode part 3: logging and billing support (env…
f08647b 
…oyproxy#34960)

There is no response in observability mode by design, so logging on onFinishProcessorCalls which is invoked on response won't work. Note, existing error/close case still work as normal

Solution: Leverage the encodeComplete function to write the logging/billing info. This is also before the consumers (extensions/filters behind) reading it.

Test: Existing Integration test frame with logging_test_filter as consumer
Risk level: Low

Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: Fernando Cainelli <fernando.cainelli-external@getyourguide.com>
cainelli pushed a commit to cainelli/envoy that referenced this pull request Jul 5, 2024
@tyxia @cainelli
ext_proc: observability mode part 3: logging and billing support (env…
c6eea9b 
…oyproxy#34960)

There is no response in observability mode by design, so logging on onFinishProcessorCalls which is invoked on response won't work. Note, existing error/close case still work as normal

Solution: Leverage the encodeComplete function to write the logging/billing info. This is also before the consumers (extensions/filters behind) reading it.

Test: Existing Integration test frame with logging_test_filter as consumer
Risk level: Low

Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: Fernando Cainelli <fernando.cainelli-external@getyourguide.com>
cainelli pushed a commit to cainelli/envoy that referenced this pull request Jul 5, 2024
@tyxia @cainelli
ext_proc: observability mode part 3: logging and billing support (env…
6e0d4c3 
…oyproxy#34960)

There is no response in observability mode by design, so logging on onFinishProcessorCalls which is invoked on response won't work. Note, existing error/close case still work as normal

Solution: Leverage the encodeComplete function to write the logging/billing info. This is also before the consumers (extensions/filters behind) reading it.

Test: Existing Integration test frame with logging_test_filter as consumer
Risk level: Low

Signed-off-by: tyxia <tyxia@google.com>
Signed-off-by: Fernando Cainelli <fernando.cainelli-external@getyourguide.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@htuch htuch htuch approved these changes

@mattklein123 mattklein123 Awaiting requested review from mattklein123 mattklein123 is a code owner

@yanavlasov yanavlasov Awaiting requested review from yanavlasov yanavlasov is a code owner

Assignees

@yanavlasov yanavlasov

@htuch htuch

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tyxia @htuch @yanavlasov