Fix(messages): set per-message tool_source for issue/PR comments (fix 2545)

[Sukuna0007Abhi]

Fixes #2545

Signed commits

• Yes, I signed my commits.

[shlokgilda]

The logic looks correct, but two things before we merge:

1. Magic strings: "Issue comment task" and "Pr comment task" are hardcoded here and probably in other places too. Should define constants to avoid typos:

TOOL_SOURCE_ISSUE_COMMENT = "Issue comment task"
TOOL_SOURCE_PR_COMMENT = "Pr comment task"

2. Historical data: This fixes new messages going forward, but what about existing data? The entire message table currently has incorrect tool_source values for issue comments. Should we include a migration script to fix old data? Or open a follow-up issue to track it?

Would love for other maintainers to chime in here.

[Sukuna0007Abhi]

Thanks @shlokgilda for the review and pls @MoralCode any thoughts?

[MoralCode]

As noted in the underlying issue, i want a better understanding of the purpose of the tool_source and tool_version fields before we make any fixes or change the way augur is logging data. That will significantly affect Shloks first suggestion.

As for historical data: i think its probably fine to leave this as a forward-looking change - its just a simple metadata field and trying to retroactively fix stuff is more of a data corruption risk than leaving it.

My personal preference would be to close this PR until we are a) past chaosscon and have the PR backlog under control, and b) have a better understanding of the intended goal of these fields so we can properly update them (not just in this task, but in other places too, such as #3486)

[sgoggins]

LGTM!

[sgoggins]

The logic looks correct, but two things before we merge:

1. **Magic strings:** `"Issue comment task"` and `"Pr comment task"` are hardcoded here and probably in other places too. Should define constants to avoid typos:

TOOL_SOURCE_ISSUE_COMMENT = "Issue comment task"
TOOL_SOURCE_PR_COMMENT = "Pr comment task"

2. **Historical data:** This fixes new messages going forward, but what about existing data? The entire message table currently has incorrect `tool_source` values for issue comments. Should we include a migration script to fix old data? Or open a follow-up issue to track it?

Would love for other maintainers to chime in here.

Hi @shlokgilda : I don't think there is a way to fix historical data without a one time "on Augur start" style of script. From there we could discern their source based on where the bridge entity record for the message exists: pull_request_review_msg_ref, pull_request_msg_ref and issue_msg_ref ... there will only ever be a record in one of those bridge entities, revealing the origins. This is how I already use these tables in analaysis. FWIW.

[sgoggins]

As noted in the underlying issue, i want a better understanding of the purpose of the tool_source and tool_version fields before we make any fixes or change the way augur is logging data. That will significantly affect Shloks first suggestion.

As for historical data: i think its probably fine to leave this as a forward-looking change - its just a simple metadata field and trying to retroactively fix stuff is more of a data corruption risk than leaving it.

My personal preference would be to close this PR until we are a) past chaosscon and have the PR backlog under control, and b) have a better understanding of the intended goal of these fields so we can properly update them (not just in this task, but in other places too, such as #3486)

tool source == Which Augur collector/task/worker inserted the data
tool version == the version of that tool

We typically only change the version when there is a significant change to its collection logic (i.e., API changes, or refactoring of collection tools).

[sgoggins]

LGTM