Feat: cloud integrations: agent check-in api

[raj-k-singh]

Summary

Details out the QS API to be consumed by AWS integration agent

Related Issues / PR's

Contributes to #6544

---

Important

Add AWS integration agent check-in API with telemetry collection strategy and update tests for service configurations.

• API Changes:
  • Add CheckInAsAgent function in controller.go to handle agent check-ins and return integration config.
  • Modify GenerateConnectionUrl in controller.go to support AWS cloud provider.
• Telemetry Strategy:
  • Introduce CloudTelemetryCollectionStrategy in model.go to manage telemetry collection for AWS.
  • Add AddServiceStrategy method to accumulate service strategies.
• Service Definitions:
  • Update integration.json for ec2 and rds to include telemetry collection strategies.
• Tests:
  • Add tests in controller_test.go for agent check-in and service configuration.
  • Update signoz_cloud_integrations_test.go to test AWS integration lifecycle and service configurations.

^{This description was created by for b6b7112. It will automatically update as commits are pushed.}

[ellipsis-dev]

👍 Looks good to me! Reviewed everything up to e6027b1 in 1 minute and 21 seconds

More details

• Looked at 804 lines of code in 8 files
• Skipped 1 files when reviewing.
• Skipped posting 5 drafted comments based on config settings.

1. pkg/query-service/app/cloudintegrations/model.go:141

• Draft comment:
  Consider using interface{} instead of any for the Scan method parameter to adhere to Go's idiomatic style. This applies to other similar instances in the code as well.
• Reason this comment was not posted:
  Confidence changes required: 30%
  The code uses any as a type, which is not idiomatic in Go. The idiomatic way is to use interface{} for empty interfaces. This is a minor issue but should be corrected for consistency and readability.

2. pkg/query-service/app/cloudintegrations/model.go:229

• Draft comment:
  Consider using interface{} instead of any for the Scan method parameter to adhere to Go's idiomatic style. This applies to other similar instances in the code as well.
• Reason this comment was not posted:
  Confidence changes required: 30%
  The code uses any as a type, which is not idiomatic in Go. The idiomatic way is to use interface{} for empty interfaces. This is a minor issue but should be corrected for consistency and readability.

3. pkg/query-service/app/cloudintegrations/model.go:297

• Draft comment:
  Consider using interface{} instead of any for the Scan method parameter to adhere to Go's idiomatic style. This applies to other similar instances in the code as well.
• Reason this comment was not posted:
  Confidence changes required: 30%
  The code uses any as a type, which is not idiomatic in Go. The idiomatic way is to use interface{} for empty interfaces. This is a minor issue but should be corrected for consistency and readability.

4. pkg/query-service/app/cloudintegrations/model.go:315

• Draft comment:
  Consider using interface{} instead of any for the Scan method parameter to adhere to Go's idiomatic style. This applies to other similar instances in the code as well.
• Reason this comment was not posted:
  Confidence changes required: 30%
  The code uses any as a type, which is not idiomatic in Go. The idiomatic way is to use interface{} for empty interfaces. This is a minor issue but should be corrected for consistency and readability.

5. pkg/query-service/app/cloudintegrations/serviceDefinitions/aws/rds/integration.json:4

• Draft comment:
  Avoid using inline styles in JSON configurations. Consider using external stylesheets or CSS classes for styling.
• Reason this comment was not posted:
  Comment did not seem useful.

Workflow ID: wflow_X2oxKruFg0XmcdoK

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on 15d37cc in 46 seconds

More details

• Looked at 234 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 11 drafted comments based on config settings.

1. pkg/query-service/app/cloudintegrations/controller.go:265

• Draft comment:
  Good job renaming MetricsCollectionConfig to MetricsCollectionStrategy for consistency. Verify that the error message now matches the new naming (e.g., 'couldn't accumulate metrics collection strategy for svc %s: %w').
• Reason this comment was not posted:
  Comment did not seem useful.

2. pkg/query-service/app/cloudintegrations/controller.go:275

• Draft comment:
  The renaming for logs from LogsCollectionConfig to LogsCollectionStrategy is clear and consistent. Confirm that the error message now reflects the new naming as intended.
• Reason this comment was not posted:
  Marked as duplicate.

3. pkg/query-service/app/cloudintegrations/model.go:240

• Draft comment:
  Renaming struct fields from '*Config' to '*Strategy' is implemented consistently, including JSON tags. Double-check that these changes do not break any external integrations that rely on the field names.
• Reason this comment was not posted:
  Marked as duplicate.

4. pkg/query-service/app/cloudintegrations/model.go:327

• Draft comment:
  The UpdateWithServiceStrategy method in AWSMetricsCollectionStrategy properly uses append to merge stream filters. The nil check and type assertion look correct.
• Reason this comment was not posted:
  Comment did not seem useful.

5. pkg/query-service/tests/integration/signoz_cloud_integrations_test.go:232

• Draft comment:
  Test cases have been updated to reflect new field names (e.g., MetricsCollectionStrategy). Ensure that the deserialization of telemetryCollectionStrategy in tests remains robust and covers edge cases.
• Reason this comment was not posted:
  Comment did not seem useful.

6. pkg/query-service/app/cloudintegrations/controller.go:265

• Draft comment:
  Renamed field: 'MetricsCollectionConfig' is now 'MetricsCollectionStrategy' in the update call. Ensure all consumers are updated accordingly.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

7. pkg/query-service/app/cloudintegrations/controller.go:275

• Draft comment:
  Renamed field: 'LogsCollectionConfig' is now 'LogsCollectionStrategy'. The logs update call reflects this change consistently.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

8. pkg/query-service/app/cloudintegrations/model.go:237

• Draft comment:
  Renamed struct fields in CloudTelemetryCollectionStrategy: both 'MetricsCollectionConfig' and 'LogsCollectionConfig' have been updated to 'MetricsCollectionStrategy' and 'LogsCollectionStrategy' respectively for consistency.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

9. pkg/query-service/app/cloudintegrations/model.go:315

• Draft comment:
  In AWSMetricsCollectionStrategy.UpdateWithServiceStrategy, variable names were updated (e.g. 'svcConfig' → 'svcStrategy', 'metricsConf' → 'metricsStrategy') to match the new naming conventions. Looks consistent.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

10. pkg/query-service/app/cloudintegrations/model.go:359

• Draft comment:
  Similarly, AWSLogsCollectionStrategy.UpdateWithServiceStrategy has been updated to use the new naming ('svcStrategy' and 'logsStrategy'). This ensures consistency across both metrics and logs strategies.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

11. pkg/query-service/tests/integration/signoz_cloud_integrations_test.go:230

• Draft comment:
  Tests have been updated to use the new telemetry collection strategy field names. Verify that these changes correctly assert the expected behavior, especially in scenarios where service configs modify the telemetry strategy.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

Workflow ID: wflow_CgglhrOgNBUDB5BQ

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[srikanthccv]

There is an extra boilerplate around, custom unmarshaling/marshaling and type‐switch code. How about using the concrete types instead of interface?

type CloudTelemetryCollectionStrategy struct {
    Provider   string                          `json:"provider"`
    AWSMetrics *AWSMetricsCollectionStrategy   `json:"aws_metrics,omitempty"`
    AWSLogs    *AWSLogsCollectionStrategy      `json:"aws_logs,omitempty"`
}

func NewAWSCloudTelemetryCollectionStrategy() *CloudTelemetryCollectionStrategy {
    return &CloudTelemetryCollectionStrategy{
        Provider: "aws",
        AWSMetrics: &AWSMetricsCollectionStrategy{
            CloudwatchMetricsStreamFilters: []CloudwatchMetricStreamFilter{},
        },
        AWSLogs: &AWSLogsCollectionStrategy{
            CloudwatchLogsSubscriptions: []CloudwatchLogsSubscriptionConfig{},
        },
    }
}

Or if you prefer to keep it same keys regardless of the provider, we could store it in raw message and then unmarshal based on provider?

type CloudTelemetryCollectionStrategy struct {
    Provider string          `json:"provider"`
    RawMetrics json.RawMessage `json:"metrics"`
    RawLogs    json.RawMessage `json:"logs"`

    AWSMetricsStrategy *AWSMetricsCollectionStrategy `json:"-"`
    AWSLogsStrategy    *AWSLogsCollectionStrategy    `json:"-"`
    GCPMetricsStrategy *GCPMetricsCollectionStrategy `json:"-"`
    GCPLogsStrategy    *GCPLogsCollectionStrategy    `json:"-"`
}

func (cts *CloudTelemetryCollectionStrategy) UnmarshalJSON(data []byte) error {
    type Alias CloudTelemetryCollectionStrategy
    aux := &struct{ *Alias }{Alias: (*Alias)(cts)}
    if err := json.Unmarshal(data, aux); err != nil {
        return err
    }

    switch cts.Provider {
    case "aws":
        if len(cts.RawMetrics) > 0 {
            var m AWSMetricsCollectionStrategy
            if err := json.Unmarshal(cts.RawMetrics, &m); err != nil {
                return err
            }
            cts.AWSMetricsStrategy = &m
        }
        if len(cts.RawLogs) > 0 {
            var l AWSLogsCollectionStrategy
            if err := json.Unmarshal(cts.RawLogs, &l); err != nil {
                return err
            }
            cts.AWSLogsStrategy = &l
        }
    default:
        return fmt.Errorf("unsupported provider: %s", cts.Provider)
    }
    return nil
}

I think this is easier to follow than interface with custom ser/de-ser around. What do you think?

[ellipsis-dev]

❌ Changes requested. Incremental review on ef50e6d in 1 minute and 19 seconds

More details

• Looked at 483 lines of code in 6 files
• Skipped 0 files when reviewing.
• Skipped posting 4 drafted comments based on config settings.

1. pkg/query-service/app/cloudintegrations/controller.go:267

• Draft comment:
  REVIEW: The error returned by AddServiceStrategy is ignored. Once fixed to return nil on success, consider handling the error to avoid silent failures.
• Reason this comment was not posted:
  Marked as duplicate.

2. pkg/query-service/app/cloudintegrations/model.go:266

• Draft comment:
  The function AddServiceStrategy always returns an error—even for supported providers (e.g. 'aws'). If the intention is to successfully accumulate strategies when the provider is supported, it should return nil instead of an error.
• Reason this comment was not posted:
  Marked as duplicate.

3. pkg/query-service/app/cloudintegrations/controller.go:267

• Draft comment:
  The call to AddServiceStrategy in CheckInAsAgent ignores its error return. If AddServiceStrategy can fail, its error should be handled rather than silently discarded.
• Reason this comment was not posted:
  Marked as duplicate.

4. pkg/query-service/app/cloudintegrations/availableServices.go:170

• Draft comment:
  Removal of the specialized JSON parsing logic for 'telemetry_collection_strategy' now relies on ParseStructWithJsonTagsFromMap with DisallowUnknownFields. Ensure that the JSON schema in service definitions exactly matches the CloudTelemetryCollectionStrategy struct to avoid unmarshalling errors.
• Reason this comment was not posted:
  Comment did not seem useful.

Workflow ID: wflow_t1nahrlu1yjZmonj

---

Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

[ellipsis-dev]

👍 Looks good to me! Incremental review on b6b7112 in 31 seconds

More details

• Looked at 32 lines of code in 2 files
• Skipped 0 files when reviewing.
• Skipped posting 4 drafted comments based on config settings.

1. pkg/query-service/app/cloudintegrations/controller.go:267

• Draft comment:
  Good error check: assigning and handling the error from AddServiceStrategy appropriately.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

2. pkg/query-service/app/cloudintegrations/model.go:262

• Draft comment:
  Explicitly returning nil in the AWS branch is a clear improvement to avoid unintended fallthrough.
• Reason this comment was not posted:
  Confidence changes required: 0%
  None

3. pkg/query-service/app/cloudintegrations/controller.go:267

• Draft comment:
  Good improvement handling the error from AddServiceStrategy. Consider confirming that svcDetails.TelemetryCollectionStrategy is guaranteed non-nil before calling it to prevent any potential nil dereference.
• Reason this comment was not posted:
  Confidence changes required: 33%
  None

4. pkg/query-service/app/cloudintegrations/model.go:246

• Draft comment:
  Consider adding a nil-check for svcStrategy at the beginning of AddServiceStrategy (e.g., if svcStrategy == nil { return nil }) to avoid a potential nil pointer panic when accessing svcStrategy.Provider.
• Reason this comment was not posted:
  Comment was not on a location in the diff, so it can't be submitted as a review comment.

Workflow ID: wflow_vCeDgPogMDom461W

---

You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.