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

fix: save and restore pb state #9606

Merged
merged 4 commits into from
Jun 7, 2023
Merged

Conversation

kingluo
Copy link
Contributor

@kingluo kingluo commented Jun 6, 2023

Description

Fixes #9270

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

leslie-tsang
leslie-tsang previously approved these changes Jun 7, 2023
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a opentelemetry or grpc-transcode problem, add the test cases to opentelemetry2.t or grpc-transcode3.t, no need to create a new test file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it doesn't belong to any plugin, this is a common pb status issue with any plugin. It's best to use a stand-alone test file to highlight this issue.

Comment on lines 102 to 104
local pb_old_state = pb.state(proto.pb_state)
local ret = handle_error_response(status_detail_type)
pb.state(pb_old_state)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not do this in handle_error_response function, around pd.decode like other place.

Comment on lines +26 to +29
- example-plugin
- key-auth
- opentelemetry
plugin_attr:
Copy link
Contributor

@monkeyDluffy6017 monkeyDluffy6017 Jun 7, 2023

Choose a reason for hiding this comment

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

Don't you need the grpc-transcode plugin to reproduce the problem?

Copy link
Contributor Author

@kingluo kingluo Jun 7, 2023

Choose a reason for hiding this comment

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

No.

local compile_proto = require("apisix.plugins.grpc-transcode.proto").compile_proto

Mark my last words. It does not depend on a specific plugin. Instead, it's a general problem with lua-protobuf lib usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

help request: opentelemetry + grpc-transcode not working
3 participants