chore: Use UploadFileV2Context due to files.upload API deprecation

[mung9]

Background

SlackReporter currently uploads files to Slack channels using the UploadFileContext function from slack-go/slack. This function internally calls the files.upload web API, but this API has been deprecated, leading to two key implications:

• From May 8, 2024, newly created Slack apps will no longer be able to use this API.
• From March 11, 2025, all Slack apps will be unable to use this API.

This PR updates the file upload method to support new Slack apps and addresses the upcoming deprecation.

The official documentation explains the deprecation as follows:

files.upload is deprecated and will stop functioning on March 11, 2025. Use files.getUploadURLExternal and files.completeUploadExternal for file uploads instead. Newly created apps will be unable to use files.upload starting May 8, 2024. See Uploading files for more details on the process, and check out this changelog for more on the deprecation.

Changes

1. Updated to use UploadFileV2Context instead of UploadFileContext.

The function has been updated to call UploadFileV2Context, which implements the latest file upload method.
Since the old version didn’t support this, the slack-go/slack version was upgraded to 0.14.0 (Latest). This version supports Go 1.16, so there are no compatibility issues with the Go language version.

The v2 function differs from the old version in two significant ways:

1. The v2 function requires the Slack channel’s ID to upload a file. You can no longer pass the channel name.
2. The v2 function requires the size of the file being uploaded to be specified.

These differences required some additional changes:

2. Modified the SlackReporter constructor to accept a Channel ID.

Previously, when creating a SlackReporter, you could pass either a channel name or ID. Now, the Channel parameter has been deprecated, and a ChannelID parameter has been added instead.

3. Updated the ReportXXProfile functions in the Reporter interface to accept file size as a parameter.

Previously, to upload profile information, you only needed to pass an io.Reader. Now, you must also specify the file size.

Migration guide

Finding the Channel ID

You can find the channel ID by right-clicking the channel name and opening the details page.

Urgency

I hope to have the review completed by October 11th.

[mung9]

I’m trying to add @mingrammer as a reviewer, but they are not showing up in the reviewer search. Could you please check and add them if you can find them? Thank you!

[mung9]

hello @mung9. Is it possible to rewrite the description by English? This repo is public, so that would be nice.

I’ve updated the descrption and a comment

[MeteorSis]

I think it would be better to deprecate the old SlackReporter and separate it into SlackReporterV2.
If there's a breaking change, We'll have to update the major version with the autopprof release, and I think it would be tiring to raise the major version because of subdependencies.
Actually, github.com/slack-go/slack also created UploadFileV2Context and uploaded the minor version, not the major version.
I'm curious to see what other people think.

#18 (comment)

[mingrammer]

@mung9 I'm on about a one-year leave, so I'm not a member of daangn org at the moment 😅. But, of course, I can review the changes, and I'm happy to see that this pkg is being maintained well. I will leave the comments soon.

[mingrammer]

Suggested change

	ReportCPUProfile(ctx context.Context, r io.Reader, size int, ci CPUInfo) error
	ReportCPUProfile(ctx context.Context, r io.Reader, fileSize int, ci CPUInfo) error

A more explicit name is better, I think. size leads the reader to be confused about what size is for.

[winterjung]

Is it necessary to expose the size argument to the interface? How about just accepting an io.Reader as before, and using io.TeeReader or io.Copy to measure the size internally?

[mingrammer]

How about just accepting an io.Reader as before, and using io.TeeReader or io.Copy to measure the size internally?

Sounds good.

[mung9]

I originally made the file size a parameter because I was concerned about excessive memory usage with large amounts of data being read using io.Reader. However, if we can guarantee that the data passed via io.Reader is small enough to be safely loaded into memory, it would actually be better to use the []byte type instead.

Meanwhile, @winterjung mentioned in the office that making breaking changes to the interface could cause issues for places where Reporter is used directly. I thought providing a migration guide for those cases would be sufficient, but I’d like to know if there are any additional concerns

[winterjung]

@mingrammer Gentle reminder.
IMHO it could be better to avoid breaking changes.

[mingrammer]

Profile file size is up to a couple of megabytes, so we don't have to worry about loading into memory. Keeping the interface and calculating the size in Reporter implementation would be a better choice.

[mingrammer]

How about leaving the channel parameter and commenting on the deprecation with the official deprecation link? And, call the new Slack API by checking whether the channelD is set, or call the legacy API if not.

This can prevent breaking changes and induce users to update their codes.

[MeteorSis]

this sounds better.
UploadFileContext is deprecated in the latest slack package, not gone.

[mung9]

I like this idea too! I’ll go with this approach

[mung9]

00db77c
I've updated params to include both Channel and ChannelID and added comments.

[mung9]

I’ve added a private function that calls the latest API based on whether ChannelID is set, so please take a look

[mung9]

@mung9 I'm on about a one-year leave, so I'm not a member of daangn org at the moment 😅. But, of course, I can review the changes, and I'm happy to see that this pkg is being maintained well. I will leave the comments soon.

@mingrammer You’ve been away for a long time! Thank you for taking the time to review despite that.