Skip to content

feat: support postgres query hook#51

Merged
cfc4n merged 1 commit into
gojue:masterfrom
yihong0618:postgres_probe
May 3, 2022
Merged

feat: support postgres query hook#51
cfc4n merged 1 commit into
gojue:masterfrom
yihong0618:postgres_probe

Conversation

@yihong0618
Copy link
Copy Markdown
Contributor

@yihong0618 yihong0618 commented May 2, 2022 •
edited
Loading

Fix:#48 basic support for Postgres 10+ will do the return value in another PR if could

image

cfc4n
cfc4n reviewed May 2, 2022
View reviewed changes
Comment thread cli/cmd/postgres.go Outdated

func init() {
postgresCmd.PersistentFlags().StringVarP(&postgresConfig.PostgresPath, "postgres", "m", "/usr/bin/postgres", "postgres binary file path, use to hook")
postgresCmd.PersistentFlags().Uint64VarP(&postgresConfig.Offset, "offset", "", 0, "0x710410")
Copy link
Copy Markdown
Member

@cfc4n cfc4n May 2, 2022

Choose a reason for hiding this comment

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

These values should not be constant. Why not use symbol name?

Copy link
Copy Markdown
Contributor Author

@yihong0618 yihong0618 May 2, 2022

Choose a reason for hiding this comment

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

will take a look and change it ~

Copy link
Copy Markdown
Contributor Author

@yihong0618 yihong0618 May 2, 2022

Choose a reason for hiding this comment

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

And I think I should drop offset

cfc4n
cfc4n reviewed May 2, 2022
View reviewed changes
Comment thread kern/postgres_kern.c Outdated
@@ -0,0 +1,42 @@
#include "ecapture.h"
#include "common.h"
Copy link
Copy Markdown
Member

@cfc4n cfc4n May 2, 2022

Choose a reason for hiding this comment

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

It was included in ecapture.h

Copy link
Copy Markdown
Contributor Author

@yihong0618 yihong0618 May 2, 2022

Choose a reason for hiding this comment

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

copy that

cfc4n
cfc4n reviewed May 2, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@cfc4n cfc4n left a comment •
edited
Loading

Choose a reason for hiding this comment

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

How to know that It works with postgres binrary? Can you give some screenshots of catrure with postgres?

@cfc4n cfc4n added the enhancement New feature or request label May 2, 2022
cfc4n
cfc4n reviewed May 2, 2022
View reviewed changes
Comment thread kern/postgres_kern.c
// versions 10 - now
// static void exec_simple_query(const char *query_string)
SEC("uprobe/exec_simple_query")
int postgres_query(struct pt_regs *ctx) {
Copy link
Copy Markdown
Member

@cfc4n cfc4n May 2, 2022

Choose a reason for hiding this comment

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

what is the difference between exec_simple_query and exec_parse_message, Why not exec_parse_message?

Copy link
Copy Markdown
Contributor Author

@yihong0618 yihong0618 May 2, 2022

Choose a reason for hiding this comment

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

exec_parse_message called in exec_simple_query.

cfc4n
cfc4n reviewed May 2, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@cfc4n cfc4n left a comment

Choose a reason for hiding this comment

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

portgres use two function to exec queries in https://github.com/postgres/postgres/blob/7b7ed046cb2ad9f6efac90380757d5977f0f563f/src/backend/tcop/postgres.c#L4539 And https://github.com/postgres/postgres/blob/7b7ed046cb2ad9f6efac90380757d5977f0f563f/src/backend/tcop/postgres.c#L4571 whit params Q and P, why is that ? and this PR can capture all query commands?

@yihong0618
Copy link
Copy Markdown
Contributor Author

yihong0618 commented May 2, 2022

How to know that It works with postgres binrary? Can you give some screenshots of catrure with postgres?

Done, the screenshoot in the content now.

@yihong0618
Copy link
Copy Markdown
Contributor Author

yihong0618 commented May 2, 2022 •
edited
Loading

portgres use two function to exec queries in https://github.com/postgres/postgres/blob/7b7ed046cb2ad9f6efac90380757d5977f0f563f/src/backend/tcop/postgres.c#L4539 And https://github.com/postgres/postgres/blob/7b7ed046cb2ad9f6efac90380757d5977f0f563f/src/backend/tcop/postgres.c#L4571 whit params Q and P, why is that ? and this PR can capture all query commands?

yes, it capture all the query I think its the same logic as mysqld

And FYI, we can not hook exec_parse_message in bin/postgres if we did not add the trace
image

@yihong0618
Copy link
Copy Markdown
Contributor Author

yihong0618 commented May 2, 2022

@cfc4n Done and clean some code.

@cfc4n
Copy link
Copy Markdown
Member

cfc4n commented May 2, 2022

please merge all commits into one commit with command

git reset --soft HEAD~3
git commit
git push -f

cfc4n
cfc4n reviewed May 2, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@cfc4n cfc4n left a comment

Choose a reason for hiding this comment

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

refer : https://www.internalpointers.com/post/squash-commits-into-one-git

@yihong0618
Copy link
Copy Markdown
Contributor Author

yihong0618 commented May 3, 2022

Of course I did that in last PR. #15 (comment)

And I think the the repo owner can use squash and merge which can also merge all commit into one.

@yihong0618
Copy link
Copy Markdown
Contributor Author

yihong0618 commented May 3, 2022

Done

@cfc4n
Copy link
Copy Markdown
Member

cfc4n commented May 3, 2022

LGTM, thanks.

@cfc4n cfc4n merged commit fe5ca3a into gojue:master May 3, 2022
@yihong0618
Copy link
Copy Markdown
Contributor Author

yihong0618 commented May 3, 2022

LGTM, thanks.

Will do some test on other env and add it to README

@yihong0618 yihong0618 mentioned this pull request Mar 21, 2023
Open
@kemingy kemingy mentioned this pull request Dec 9, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cfc4n cfc4n cfc4n left review comments

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@yihong0618 @cfc4n