Skip to content
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

feature: support for batch creating task through http_sd #2018

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

Calvin979
Copy link
Contributor

@Calvin979 Calvin979 commented May 20, 2024

What's changed?

Steps to batch create task through http_sd

  1. Add http_sd params in template yml
  - field: http_sd
    name:
      zh-CN: http_sd
      en-US: http_sd
    type: text
    placeholder: Enter http_sd url
    required: false
    hide: true
  1. Enter http_sd url when trying to create task(optional)

001addmonitor
002httpsd
003monitorlist

Checklist

  • I have read the Contributing Guide
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Add or update API

  • I have added the necessary e2e tests and all cases have passed.

@tomsun28
Copy link
Contributor

👍 relate pr #1791

@tomsun28 tomsun28 added the good first pull request Good for newcomers label May 20, 2024
@tomsun28 tomsun28 self-requested a review May 20, 2024 15:02
@Calvin979
Copy link
Contributor Author

Relate issue #1625

@tomsun28 tomsun28 added enhancement New feature or request new feature labels May 21, 2024
@tomsun28 tomsun28 added this to the 1.6.1 milestone May 21, 2024
@tomsun28
Copy link
Contributor

hi thanks for contribution. there are some conflict files need be resolved if you are time.

# Conflicts:
#	collector/src/main/java/org/apache/hertzbeat/collector/dispatch/DispatchConstants.java
#	collector/src/main/resources/META-INF/services/org.apache.hertzbeat.collector.collect.AbstractCollect
#	common/src/main/java/org/apache/hertzbeat/common/entity/job/Metrics.java
#	common/src/main/java/org/apache/hertzbeat/common/entity/manager/Monitor.java
#	manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java
@Calvin979
Copy link
Contributor Author

hi thanks for contribution. there are some conflict files need be resolved if you are time.

@tomsun28
I've resolved conflict, please wait me to check the code and feature if is all set before merging.

# Conflicts:
#	manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java
@Calvin979
Copy link
Contributor Author

@tomsun28
This branch can be merged now!

@tomsun28
Copy link
Contributor

@tomsun28 This branch can be merged now!

thanks, we want to merge this after publish v1.6.0

@Calvin979
Copy link
Contributor Author

@tomsun28 This branch can be merged now!

thanks, we want to merge this after publish v1.6.0

Got it.

@tomsun28 tomsun28 linked an issue Jun 12, 2024 that may be closed by this pull request
@tomsun28
Copy link
Contributor

tomsun28 commented Jul 8, 2024

hi, how about the design when update the monitoring.

@Calvin979
Copy link
Contributor Author

hi, how about the design when update the monitoring.

Do you mean updating http_sd monitor or those monitors which are created by http_sd?

@tomsun28
Copy link
Contributor

tomsun28 commented Jul 8, 2024

Do you mean updating http_sd monitor or those monitors which are created by http_sd?

hi, yes. Due the monitors automatic discovery by http_sd response data, when http_sd response data change, should we consider updating the automatically created monitors?

@tomsun28
Copy link
Contributor

tomsun28 commented Jul 8, 2024

Suggest we consider these designs in the first version to facilitate future expansion, but do not have to implement it right away.

@Calvin979
Copy link
Contributor Author

Calvin979 commented Jul 8, 2024

Do you mean updating http_sd monitor or those monitors which are created by http_sd?

hi, yes. Due the monitors automatic discovery by http_sd response data, when http_sd response data change, should we consider updating the automatically created monitors?

It should be discussed first.
I think these steps are the key of updating monitors automatically:

  1. The monitor which is configed http_sd(Maybe we can refer to it as the main monitor below) must have a relation in DB with monitors which are created by the main monitor. So that we can handle each monitor with the main monitor.
    image
  2. Create a cyclic task for the main monitor, which is used to collect service discovery data.
  3. Isolate data in CommonDataQueue due to service discovery data is completely different from other data.
  4. Manager poll service discovery data and update monitors.
    image

So far this idea came into my mind. How about this design in general?

@tomsun28
Copy link
Contributor

tomsun28 commented Jul 9, 2024

hi, look great!!

  1. The monitor which is configed http_sd(Maybe we can refer to it as the main monitor below) must have a relation in DB with monitors which are created by the main monitor. So that we can handle each monitor with the main monitor.

It is recommended to create a new table to associate their relationships.
Not sure if we need to add an 'auto' field to 'hzb_monitor' to indicate if this monitor was created by auto discover or not, if it's not non-essential, we may choose not add it now.

The design makes full use of existing capabilities while minimizing modifications.👍

@Calvin979
Copy link
Contributor Author

hi, look great!!

  1. The monitor which is configed http_sd(Maybe we can refer to it as the main monitor below) must have a relation in DB with monitors which are created by the main monitor. So that we can handle each monitor with the main monitor.

It is recommended to create a new table to associate their relationships. Not sure if we need to add an 'auto' field to 'hzb_monitor' to indicate if this monitor was created by auto discover or not, if it's not non-essential, we may choose not add it now.

The design makes full use of existing capabilities while minimizing modifications.👍

We can add a tag named Auto created by http_sd to each monitor.
I will try to implement this design this weekend.

# Conflicts:
#	manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java
@Calvin979
Copy link
Contributor Author

@tomsun28
http_sd can detect target regularly and create monitor automatically now.
image

@Calvin979
Copy link
Contributor Author

Two steps to support more sd if we need it:

  1. Create a sd collect like any other collect
    image
  2. Add an enum element in org.apache.hertzbeat.common.entity.sd.ServiceDiscoveryProtocol
    image

@Calvin979
Copy link
Contributor Author

The entire process of sd:
image
@tomsun28

@Calvin979
Copy link
Contributor Author

hi, look great!!

  1. The monitor which is configed http_sd(Maybe we can refer to it as the main monitor below) must have a relation in DB with monitors which are created by the main monitor. So that we can handle each monitor with the main monitor.

It is recommended to create a new table to associate their relationships. Not sure if we need to add an 'auto' field to 'hzb_monitor' to indicate if this monitor was created by auto discover or not, if it's not non-essential, we may choose not add it now.

The design makes full use of existing capabilities while minimizing modifications.👍

I create a table named hzb_monitor_bind to associate relationships.
See also: org.apache.hertzbeat.common.entity.manager.MonitorBind

Calvin979 and others added 15 commits July 19, 2024 21:16
# Conflicts:
#	manager/src/main/java/org/apache/hertzbeat/manager/controller/MonitorsController.java
#	manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java
# Conflicts:
#	collector/src/main/java/org/apache/hertzbeat/collector/collect/registry/RegistryImpl.java
#	common/src/main/java/org/apache/hertzbeat/common/config/CommonProperties.java
#	common/src/main/java/org/apache/hertzbeat/common/queue/impl/KafkaCommonDataQueue.java
#	common/src/test/java/org/apache/hertzbeat/common/queue/impl/KafkaCommonDataQueueTest.java
#	manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java
# Conflicts:
#	manager/src/main/java/org/apache/hertzbeat/manager/service/impl/MonitorServiceImpl.java
#	manager/src/test/java/org/apache/hertzbeat/manager/service/MonitorServiceTest.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

[Task] <Support discovery monitoring instance through http_sd>
3 participants