support to post in thread

[goro9]

Hello,

I added an option to post in the form of a thread reply, which I thought might be useful in an environment where multiple batches run in parallel.

Please review the changes and let me know if you have any questions or suggestions. Looking forward to your feedback.

[samber]

Can you explain the choice of setting this at the handler level, instead of record context ?

[goro9]

For example, if tasks a and b are executed at approximately the same time, I would like to separate the threads for each task and log output as follows.

In this case, I would like to specify the same ThreadTimestamp throughout the tasks, so I thought it would be better to specify it in the handler option.
(On the other hand, I couldn't think of a situation where we would want to switch between thread replies and direct posting to a channel on a record-by-record basis.)
However, it may be an unusual use, so I assume that I may use the fork version.

[samber]

I really like the idea, but starting a new logger for every thread seems very odd to me.

What about something like this ?

ctx := context.WithValue(context.Background(), "threadId", time.Now())

logger.InfoContext(ctx, "start task a")
logger.ErrorContext(ctx, "task failed")

func (SlackHandler) Handler(ctx context.Context, r *slog.Record) {
    threadId, ok := ctx.Value("threadId").(time.Time)
    // ...
}

We need to check if using a threadId that does not exist is ok for slack API, but it makes the code simpler IMO.

This is very similar to the traceId+spanId proposed by OTEL.

[goro9]

Certainly could be odd given the logger usage.
How about this fix? c598687
I checked the operation like this

[samber]

Thanks, this is much better.

Unfortunately, I made a test myself and Slack does not allow thread creation when pushing the first message with a not existing thread timestamp.

A few notes:

• Slack does not return the thread timestamp in the response to the "incoming webhook" (only in response to the REST API).
• Having to call the Slack API first, to generate that thread id seems a bad developer experience.
• It does not work if your app is replicated in production, as you cannot guess the thread id generated in another instance

I'm starting to think your first idea was much better: a hardcoded thread id, at the logger option level. No thread creation at runtime. Just push a message to the end. It would be a very limited use case, but still valid.

Any opinion on this ?

[goro9]

As you say, the timestamp of the root post must exist in order for a thread to be replied to.
Even when multiple replying to the same thread, there is only one timestamp to specify, the root one.
There seems to have been a discrepancy in this understanding. My apologies.

Slack does not return the thread timestamp in the response to the "incoming webhook" (only in response to the REST API).

As you say, we cannot get the timestamp in the webhook response, so we need to GET it separately.
Since it is difficult to take care of this in this package, I thought it is better to make it the responsibility of the main application to obtain the timestamp.

Having to call the Slack API first, to generate that thread id seems a bad developer experience.

As mentioned earlier, the main application must call the Slack API at least once to obtain a timestamp.
Personally, I consider it acceptable if it is only once when generating loggers.

It does not work if your app is replicated in production, as you cannot guess the thread id generated in another instance

I have no problem with the behavior of threads being generated per replicated application for the use case I'm envisioning.

I wrote my use case in code on how to specify the thread timestamp in the handler options
I would like to output generic logs such as start, end, etc. during task execution using middleware, etc., and then output logs to slack as needed in the main process.
goro9/zatta-cmd@c45b340

[samber]

Ok, let's merge your last commit and see how people use it in the future!

I'm going to add a comment for feedback collection.

[samber]

I just created the issue: #7

[samber]

...and added an example to the readme.

[goro9]

OK.
I will try to find a better way on my end too.
Thank you for the feedback!