-
-
Notifications
You must be signed in to change notification settings - Fork 0
Enabled analytics logging to opensearch #9145
base: dev
Are you sure you want to change the base?
Conversation
@@ -38,6 +38,9 @@ const DEFAULT_INTERVAL_SECONDS = 1800 | |||
const configInterval = parseInt(config.taskserver.processInterval) | |||
const interval = (configInterval || DEFAULT_INTERVAL_SECONDS) * 1000 | |||
|
|||
let lastTimestamp: string | |||
let analyticsData: any[] = [] | |||
const collectedData: any[] = [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will collectedData continue to grow in memory infinitely? I think it will also repeat a lot of data because of that. Unless you're filtering on the Opensource ingest side.
const currentTimestamp = new Date().toISOString() | ||
|
||
if (lastTimestamp) { | ||
analyticsData = analyticsData.filter((data) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe there is any data in analyticsData. Plus it is not being used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there's some changes needed to ensure we're not growing in memory. Plus I think the analyticsData section isn't functioning in its current state.
Summary
🤖 Generated by Copilot at c6cf091
This pull request implements analytics collection for the task server. It adds a new file
collect-analytics.ts
that handles the data processing and storage.References
closes #insert number here
Explanation
🤖 Generated by Copilot at c6cf091
packages/taskserver/src/collect-analytics.ts
)🤖 Generated by Copilot at c6cf091
QA Steps
List any additional steps required to QA the changes of this PR, as well as any supplemental images or videos.
Checklist