-
Notifications
You must be signed in to change notification settings - Fork 69
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
Integrate notifications backend #129
Merged
joshuali925
merged 19 commits into
opensearch-project:main
from
joshuali925:integrate-notifications
Oct 7, 2021
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
a96f5c4
WIP send notifications
joshuali925 4d12a59
Remove send notification when creating report definition
joshuali925 c5674ae
WIP
joshuali925 112585b
Remove unused lines
joshuali925 ca0e1d9
Move notifications actions into separate package
joshuali925 e4655a4
Revert formatting
joshuali925 d6f7c03
Merge branch 'main' into integrate-notifications
joshuali925 3c8e34e
Merge branch 'main' into integrate-notifications
joshuali925 aeffad9
Remove delivery settings to unblock CI for now
joshuali925 3271b58
Preserve thread context in coroutine
joshuali925 3a2672d
Merge branch 'main' into integrate-notifications
joshuali925 53e862a
Catch WarningFailureException to fix integTests for legacy APIs
joshuali925 14f8959
Remove unnecessary blank line
joshuali925 a79899b
Change feature type to string
joshuali925 3163d3b
Merge branch 'main' of github.com:opensearch-project/dashboards-repor…
joshuali925 22aa2db
Add integTest with notifications
joshuali925 fec2017
Merge branch 'main' into integrate-notifications
joshuali925 5871859
Remove println
joshuali925 1c30567
Add user context for background jobs
joshuali925 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
141 changes: 141 additions & 0 deletions
141
...ler/src/main/kotlin/org/opensearch/reportsscheduler/notifications/NotificationsActions.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,141 @@ | ||
/* | ||
* SPDX-License-Identifier: Apache-2.0 | ||
* | ||
* The OpenSearch Contributors require contributions made to | ||
* this file be licensed under the Apache-2.0 license or a | ||
* compatible open source license. | ||
* | ||
* Modifications Copyright OpenSearch Contributors. See | ||
* GitHub history for details. | ||
*/ | ||
|
||
package org.opensearch.reportsscheduler.notifications | ||
|
||
import org.opensearch.action.ActionListener | ||
import org.opensearch.client.node.NodeClient | ||
import org.opensearch.common.util.concurrent.ThreadContext | ||
import org.opensearch.commons.ConfigConstants | ||
import org.opensearch.commons.notifications.NotificationConstants.FEATURE_REPORTS | ||
import org.opensearch.commons.notifications.NotificationsPluginInterface | ||
import org.opensearch.commons.notifications.action.SendNotificationResponse | ||
import org.opensearch.commons.notifications.model.ChannelMessage | ||
import org.opensearch.commons.notifications.model.EventSource | ||
import org.opensearch.commons.notifications.model.SeverityType | ||
import org.opensearch.reportsscheduler.ReportsSchedulerPlugin.Companion.LOG_PREFIX | ||
import org.opensearch.reportsscheduler.model.CreateReportDefinitionResponse | ||
import org.opensearch.reportsscheduler.model.ReportDefinition | ||
import org.opensearch.reportsscheduler.util.logger | ||
|
||
/** | ||
* Report definitions index operation actions. | ||
*/ | ||
internal object NotificationsActions { | ||
private val log by logger(NotificationsActions::class.java) | ||
|
||
private lateinit var client: NodeClient | ||
|
||
/** | ||
* Initialize the class | ||
* @param client NodeClient for transport call | ||
*/ | ||
fun initialize(client: NodeClient) { | ||
this.client = client | ||
} | ||
|
||
/** | ||
* Send notifications based on delivery parameter | ||
* @param delivery [ReportDefinition.Delivery] object | ||
* @param referenceId [String] object | ||
* @return [CreateReportDefinitionResponse] | ||
*/ | ||
fun send(delivery: ReportDefinition.Delivery, referenceId: String): SendNotificationResponse? { | ||
return send(delivery, referenceId, "") | ||
} | ||
|
||
/** | ||
* Send notifications based on delivery parameter | ||
* @param delivery [ReportDefinition.Delivery] object | ||
* @param referenceId [String] object | ||
* @param userStr [String] object, | ||
* @return [CreateReportDefinitionResponse] | ||
*/ | ||
fun send(delivery: ReportDefinition.Delivery, referenceId: String, userStr: String?): SendNotificationResponse? { | ||
if (userStr.isNullOrEmpty()) { | ||
return sendNotificationHelper(delivery, referenceId) | ||
} | ||
|
||
var sendNotificationResponse: SendNotificationResponse? = null | ||
client.threadPool().threadContext.stashContext().use { | ||
client.threadPool().threadContext.putTransient( | ||
ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT, | ||
userStr | ||
) | ||
sendNotificationResponse = sendNotificationHelper(delivery, referenceId) | ||
} | ||
return sendNotificationResponse | ||
} | ||
|
||
private fun sendNotificationHelper( | ||
delivery: ReportDefinition.Delivery, | ||
referenceId: String | ||
): SendNotificationResponse? { | ||
log.info("$LOG_PREFIX:NotificationsActions-send") | ||
var sendNotificationResponse: SendNotificationResponse? = null | ||
NotificationsPluginInterface.sendNotification( | ||
client, | ||
EventSource(delivery.title, referenceId, FEATURE_REPORTS, SeverityType.INFO), | ||
ChannelMessage(delivery.textDescription, delivery.htmlDescription, null), | ||
delivery.configIds, | ||
object : ActionListener<SendNotificationResponse> { | ||
override fun onResponse(response: SendNotificationResponse) { | ||
sendNotificationResponse = response | ||
log.info("$LOG_PREFIX:NotificationsActions-send:$sendNotificationResponse") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please record response and also add metrics around success and failure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll add it in another PR, merging this to unblock others |
||
} | ||
|
||
override fun onFailure(exception: Exception) { | ||
log.error("$LOG_PREFIX:NotificationsActions-send Error:$exception") | ||
} | ||
} | ||
) | ||
return sendNotificationResponse | ||
} | ||
|
||
/** | ||
* Executes the given [block] function on this resource and then closes it down correctly whether an exception | ||
* is thrown or not. | ||
* | ||
* In case if the resource is being closed due to an exception occurred in [block], and the closing also fails with an exception, | ||
* the latter is added to the [suppressed][java.lang.Throwable.addSuppressed] exceptions of the former. | ||
* | ||
* @param block a function to process this [AutoCloseable] resource. | ||
* @return the result of [block] function invoked on this resource. | ||
*/ | ||
@Suppress("TooGenericExceptionCaught") | ||
private inline fun <T : ThreadContext.StoredContext, R> T.use(block: (T) -> R): R { | ||
var exception: Throwable? = null | ||
try { | ||
return block(this) | ||
} catch (e: Throwable) { | ||
exception = e | ||
throw e | ||
} finally { | ||
closeFinally(exception) | ||
} | ||
} | ||
|
||
/** | ||
* Closes this [AutoCloseable], suppressing possible exception or error thrown by [AutoCloseable.close] function when | ||
* it's being closed due to some other [cause] exception occurred. | ||
* | ||
* The suppressed exception is added to the list of suppressed exceptions of [cause] exception. | ||
*/ | ||
@Suppress("TooGenericExceptionCaught") | ||
private fun ThreadContext.StoredContext.closeFinally(cause: Throwable?) = when (cause) { | ||
null -> close() | ||
else -> try { | ||
close() | ||
} catch (closeException: Throwable) { | ||
cause.addSuppressed(closeException) | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
https://github.com/opensearch-project/dashboards-reports/blob/371f1d18855de49688379634fa867f3f8eaef8e4/reports-scheduler/src/main/kotlin/org/opensearch/reportsscheduler/model/ReportDefinition.kt#L53
I think by combining the
origin
+ path("_plugin/reporting/report/") +refernceId
we can get the link and add to the notification messageThere 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.
@zhongnansu the origin will always be localhost right? it's used to access UI internally, but should not be sent to user as a report link? Also this doesn't seem to include
basePath
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 doubt that it's always the localhost, let me check
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 see, how is origin used in reporting? I checked it doens't seem to include basepath, not sure if something breaks if we append basepath
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.
https://github.com/opensearch-project/dashboards-reports/blob/371f1d18855de49688379634fa867f3f8eaef8e4/dashboards-reports/server/routes/report.ts#L72
It gets origin from the request headers, which should be good. It's not used to access UI internally, it was used for the old implementation of notification where we using polling to get job, and compose the link at kibana side, then send to user.
But I agreed that it's still missing the basePath. will start work on the solution after this PR is merged