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

Feature(CORE): Activity log extension #300

Merged
merged 10 commits into from
Sep 18, 2023

Conversation

onlyjackfrost
Copy link
Contributor

Description

Add a activity log extension to send activity log like incoming request information and cache refreshing result.
Activity logs aim to provide a more specific and accurate information compared to the access log or debug logger stdout.

How to use

# vulcan.yaml
activity-log:
    enabled: true
    options:
        http-logger:
            connection:
                ssl: boolean. # use https or not
                host: string
                port: integer
                path: "url path"

After you set your project config, vulcan server will send activity log to remote server asynchronously when cache refreshing job is done and after processed an API request.

A cache refreshing activity log will contain info

logTime(timestamp in UTC) -- the time that refresh job started
urlPath -- indicate which API it belong
sql -- the sql statement that will be executed 
refreshResult -- Enum(success, failed), the result of refresh job

In an API request activity log will contain info

logTime(timestamp in UTC) -- the timestamp that server received the request
duration(ms) -- the approximate time that server handled the API request
method -- the request method
url -- the request url
ip -- the original IP
header -- the request headers
params -- the request params
query -- the request query parameters
status -- the response HTTP status
error -- the error message that VulcanSQL provided
user -- the attribute that the client user have (gain after authenticate)

Test

unit tests
Labs:
manually create a koa server on localhost with a router "(POST)/test"
vulcan.yaml:

...
activity-log:
  enabled: true
  options:
    http-logger:  
      connection:
        host: localhost
        port: 8080
        path: test
  • Activity log should be send after refresh job done

image

image
  • Activity log should be send after processed API request
    image

@vercel
Copy link

vercel bot commented Sep 11, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
vulcan-sql-document ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 18, 2023 9:26am

Copy link
Contributor

@kokokuo kokokuo left a comment

Choose a reason for hiding this comment

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

Beside some questions and suggestions, other LGTM

) {
const { urlPath } = schema;
const { sql } = cache;
// if fn is not a function, return
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the comment? Seems no fn ?

}
}

protected sendActivityLog = async (
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does here use the functional expression repsentation ( xxx = () => {} )? Is the reason to do it?

});
};

protected getUrl = (connection: HttpLoggerConnectionConfig): string => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.

throw new Error('Http logger connection should be provided');
}
const headers = option.connection.headers;
const url = this.getUrl(option.connection);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious seems you have provided a function called getUrl in utils and here you also create the same name method, but also the same code ( not using the header ), so why does not use the getUrl in utils directly?

const url = getUrl(connection as ConnectionConfig);

Comment on lines 2 to 3
ssl?: boolean | undefined;
host?: string | undefined;
Copy link
Contributor

Choose a reason for hiding this comment

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

You have added the ?, is means undefined, so actually, you don't need to write again by undefined.

path?: string | undefined;
}

export const getUrl = (connection: ConnectionConfig): string => {
Copy link
Contributor

@kokokuo kokokuo Sep 14, 2023

Choose a reason for hiding this comment

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

Same as above of path

Copy link
Contributor

@kokokuo kokokuo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

}),
{ preventOverrun: true, id: workerId }
);
// add the job to schedule cache refresh task
this.scheduler.addIntervalJob(refreshJob);
} else {
await this.cacheLoader.load(templateName, cache);
await this.sendActivityLogAfterLoad(schema, cache);
Copy link
Contributor

@kokokuo kokokuo Sep 18, 2023

Choose a reason for hiding this comment

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

Optional: Suggest renaming LoadCacheAndSendActivityLog, if you are available

@onlyjackfrost onlyjackfrost merged commit cfe52ca into develop Sep 18, 2023
2 checks passed
@hanshino hanshino deleted the feature/remote-activity-log branch January 31, 2024 07:39
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.

2 participants