Added the default ignore config for service disable error #476

ParthaI · 2023-08-07T06:53:24Z

Integration test logs

Logs

Add passing integration test logs here

Example query results

Results

Add example SQL query results here (please include the input queries as well)

cbruno10 · 2023-08-15T15:18:24Z

@johnsmyth I think this feature is helpful for ignoring errors better, especially when the error codes we get back are simple and lack details, e.g., 403, 500.

One thought I had was, would more flexibility in be helpful or too complex?

For instance:

ignore_error_configs = [
  { codes: ["400", "403"], messages: ["foo", "bar"] }, // Ignore errors with at least one of these codes and one of these messages
  { codes: ["500"], // Ignore errors with this code
  { messages: ["^.*foobar$"] // Ignore errors whose message matches this pattern
]

The main use case would be ignoring specific code/message combinations, e.g., if multiple error codes had similar messages, but typing it out, it seems a little more complex than required in this case and in most plugins.

johnsmyth · 2023-08-15T18:15:29Z

@cbruno10 You are correct that The main use case would be ignoring specific code/message combinations, e.g., if multiple error codes had similar messages. Not knowing what these error messages look like, or how common that case is, Its hard to make a recommendation. If it is uncommon / unlikely, I would probably opt for the simpler, backward-compatible solution.

cbruno10

@ParthaI Please see review comments, thanks!

cbruno10 · 2023-08-15T19:54:08Z

config/gcp.spc

@@ -20,6 +20,9 @@ connection "gcp" {
  # If not set, no impersonation is done.
  #impersonate_service_account = "YOUR_SERVICE_ACCOUNT"

+  # `ignore_error_messages` (optional) - List of additional GCP error message pattern to ignore for all queries.
+  #  ignore_error_messages = ["API has not been used", "other regex"]


@ParthaI Is there a more exact regex (using ^, $) we should add as an example here, in particular for the "API has not been used" case? We can probably remove other regex as it's not helpful.

This is a really good addition to handle cases where, google uses the same 403 error code when - API are not enabled and when Quota exceeded. An exact regex mapping example to an error message would sure be helpful.

For reference, this is reported as this:

ERROR: googleapi: Error 403: Cloud DNS API has not been used in project XXX before or it is disabled. Enable it by visiting https://console.developers.google.com/apis/api/dns.go… Details: [ { "@type": "type.googleapis.com/google.rpc.Help", "links": [ { "description": "Google developers console API activation", "url": "https://console.developers.google.com/apis/api/dns.googleapis.com/overview?project=XXX" } ] }, { "@type": "type.googleapis.com/google.rpc.ErrorInfo", "domain": "googleapis.com", "metadata": { "consumer": "projects/XXX", "service": "dns.googleapis.com" }, "reason": "SERVICE_DISABLED" } ] , accessNotConfigured (SQLSTATE HV000)

So I think the current example with API has not been used would be enough.

@cbruno10, I have added the example for the exact regex (using ^, $) i.e ignore_error_messages = ["^.*API\\s*has\\s*not\\s*been\\s*used.*$"]. Also we can pass a message just like a string format, in both of the case the code changes are working fine.

NOTE: In the .spc file, we should use \\ instead of \ for the regex pattern to avoid errors related to escape characters. Otherwise, parsing the .spc file might result in an error like "Invalid escape sequence: The symbol "d" is not a valid escape sequence selector."

cbruno10 · 2023-08-15T19:54:40Z

gcp/ignore_error_predicate.go

 		if gerr, ok := err.(*googleapi.Error); ok {
+			for _, pattern := range gcpConfig.IgnoreErrorMessages {
+				re := regexp.MustCompile(pattern)
+				result := re.FindString(types.ToString(gerr.Message))


Does GCP ever send messages that don't have the Message property? If so, does this code still handle everything ok?

I haven't encountered any errors without a message property. Additionally, we are converting errors of type googleapi.Error, which always includes a message property. I've tested it multiple times with various connection combinations, and it's working correctly, handling errors as expected.

If an error does not contain a message property, the code block above will not function as it is designed to handle errors based on their error message. In cases where the message property is not available, an alternative approach is to configure the ignore_error_codes in the .spc file. This will allow the following line of code to handle the error.

According to the GCP API design documentation, errors include a message property. However, it's important to note that Error messages are not part of the API surface. They are subject to changes without notice. Application code should not rely heavily on error messages as they may change.

cbruno10 · 2023-08-15T19:57:58Z

gcp/ignore_error_predicate.go

 		if gerr, ok := err.(*googleapi.Error); ok {
+			for _, pattern := range gcpConfig.IgnoreErrorMessages {
+				re := regexp.MustCompile(pattern)
+				result := re.FindString(types.ToString(gerr.Message))


Does this function support matching on full regular expressions? How is this different than the regex Match function?

Yes, this function support matching on full regular expressions.

*func (re Regexp) FindString(s string) string
FindString returns a string holding the text of the leftmost match in s of the regular expression. If there is no match, the return value is an empty string, but it will also be empty if the regular expression successfully matches an empty string. Use FindStringIndex or FindStringSubmatch if it is necessary to distinguish these cases.

*func (re Regexp) Match(b []byte) bool
Match reports whether the byte slice b contains any match of the regular expression re.

IMO, it would be better to use MatchString function here. I can see in most of the place we use MatchString function to validate patterns. https://github.com/search?q=repo%3Aturbot%2Fsteampipe%20MatchString&type=code

I have replace the function FindString to MatchString.

cbruno10 · 2023-08-15T19:58:55Z

gcp/ignore_error_predicate.go

+				}
+			}
+
+			if !hasIgnoredErrorCodes(d.Connection) {


Do we need this block still?

Removed this block of code as we do not need those.

…sue-286

cbruno10

@ParthaI Please see comments, thanks!

cbruno10 · 2023-10-04T15:26:44Z

config/gcp.spc

@@ -20,6 +20,9 @@ connection "gcp" {
  # If not set, no impersonation is done.
  #impersonate_service_account = "YOUR_SERVICE_ACCOUNT"

+  # `ignore_error_messages` (optional) - List of additional GCP error message pattern to ignore for all queries.
+  #  ignore_error_messages = ["^.*API has not been used.*$"]


What error does a user receive if an invalid regex is passed in?

Also, do users need to use ^ and $, or will we still match without those explicitly included in a pattern?

The user will receive an error error parsing regexp: invalid escape sequence: during the query execution if a user provide an invalid regex.

User may include ^ and $ or may not be, in both of the case the code changes are working fine.

cbruno10 · 2023-10-04T15:27:07Z

config/gcp.spc

@@ -20,6 +20,9 @@ connection "gcp" {
  # If not set, no impersonation is done.
  #impersonate_service_account = "YOUR_SERVICE_ACCOUNT"

+  # `ignore_error_messages` (optional) - List of additional GCP error message pattern to ignore for all queries.


Is an error ignored if the message matches a value from here OR if the error code matches the other config arg?

If the ignore_error_messages has been set and the pattern matches with the error message the error will be ignore from here.

cbruno10 · 2023-10-04T15:29:04Z

gcp/ignore_error_predicate.go

+			// Add to support regex match as per error message
+			for _, pattern := range gcpConfig.IgnoreErrorMessages {
+				re := regexp.MustCompile(pattern)
+				result := re.MatchString(types.ToString(gerr.Message))


Do we need to cast with types.toString? What type is the Message property?

Also, is there ever a case where Message is not included as part of an error?

Do we need to cast with types.toString? What type is the Message property?

No, we do not need to cast with types.toString, the Message property it self a string.

Also, is there ever a case where Message is not included as part of an error?

No, I have not face such type of scenario as of now.

github-actions · 2023-12-03T23:31:18Z

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions · 2024-02-02T23:31:21Z

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions · 2024-03-04T23:31:17Z

This PR was closed because it has been stalled for 90 days with no activity.

pdecat · 2024-10-16T07:33:30Z

FWIW, I've squashed and rebased the commits from this PR here: https://github.com/pdecat/steampipe-plugin-gcp/tree/issue-286, and can confirm it works great :)

I've integrated the changes in our custom build of the plugin here: https://github.com/pdecat/steampipe-plugin-gcp/tree/dev

pdecat · 2024-11-12T10:55:03Z

This PR is also working great to ignore issues when new regions are added, like is currently happening with northamerica-south1 (Queretaro):

2024-11-12 10:38:10.814 UTC [ERROR] steampipe-plugin-gcp.plugin: [ERROR] 1731407888653: gcp_cloud_run_service.listCloudRunServices:
  api_error=
  | googleapi: Error 400: region: Unsupported region northamerica-south1
  | Details:
  | [
  |   {
  |     "@type": "type.googleapis.com/google.rpc.BadRequest",
  |     "fieldViolations": [
  |       {
  |         "description": "Unsupported region northamerica-south1",
  |         "field": "region"
  |       }
  |     ]
  |   }
  | ]
  | , badRequest

The following configuration allows to ignore them (along with service disabled errors):

connection "gcp" {
  plugin = plugin.gcp

  ignore_error_messages = [
    "^.*API has not been used.*$",
    "^.*Unsupported region.*$",
  ]
}

pdecat · 2024-11-28T08:34:39Z

gcp/ignore_error_predicate.go

+
 		if gerr, ok := err.(*googleapi.Error); ok {
+
+			// Add to support regex match as per error message
+			for _, pattern := range gcpConfig.IgnoreErrorMessages {
+				re := regexp.MustCompile(pattern)
+				result := re.MatchString(gerr.Message)
+				if result {
+					return true
+				}
+			}
+


Turns out not all Google Cloud SDK APIs use the googleapi.Error type, e.g. Redis APIs, so I've altered this part of the code to compare the error message of all errors:

Suggested change

if gerr, ok := err.(*googleapi.Error); ok {

// Add to support regex match as per error message

for _, pattern := range gcpConfig.IgnoreErrorMessages {

re := regexp.MustCompile(pattern)

result := re.MatchString(gerr.Message)

if result {

return true

}

}

logger := plugin.Logger(ctx)

logger.Trace("ignore_error_predicate.shouldIgnoreErrorPluginDefault", "err.Error()", err.Error())

// Add to support regex match as per error message

for _, pattern := range gcpConfig.IgnoreErrorMessages {

logger.Trace("ignore_error_predicate.shouldIgnoreErrorPluginDefault", "message_pattern", pattern)

// TODO: maybe compile these at the connection level for performance reasons

re := regexp.MustCompile(pattern)

result := re.MatchString(err.Error())

if result {

logger.Debug("ignore_error_predicate.shouldIgnoreErrorPluginDefault", "ignore_error_message", err.Error())

return true

}

}

Without this change, API has not been used cannot be ignored for Redis API calls.

Note that this requires enabling multiline regex matching with (?s) as errors are serialized as:

2024-11-28 08:23:11.222 UTC [TRACE] steampipe-plugin-gcp.plugin: [TRACE] 1732782190328: ignore_error_predicate.shouldIgnoreErrorPluginDefault: err.Error()= | rpc error: code = PermissionDenied desc = Google Cloud Memorystore for Redis API has not been used in project my-project before or it is disabled. Enable it by visiting https://console.developers.google.com/apis/api/redis.googleapis.com/overview?project=my-project then retry. If you enabled this API recently, wait a few minutes for the action to propagate to our systems and retry. | error details: name = ErrorInfo reason = SERVICE_DISABLED domain = googleapis.com metadata = map[consumer:projects/my-project service:redis.googleapis.com] | error details: name = Help desc = Google developers console API activation url = https://console.developers.google.com/apis/api/redis.googleapis.com/overview?project=my-project

E.g.:

ignore_error_messages = [ "(?s)^.*API has not been used.*$", "(?s)^.*Unsupported region.*$", ]

Also added some logs to the error code checks right below.

Pushed the changes here: pdecat@d8b31a2

Can submit an PR targeting this PR's branch if you want.

@pdecat Yes please, we'd greatly appreciate a PR!

Submitted #688

* fix: ignore error messages from all error types

Added the default ignore config for service disable error

e1276f6

ParthaI self-assigned this Aug 7, 2023

ParthaI marked this pull request as draft August 7, 2023 06:53

Added support for ignoring errors by error message

c0187e0

ParthaI marked this pull request as ready for review August 10, 2023 11:17

rajlearner17 requested review from cbruno10 and bigdatasourav August 13, 2023 07:04

cbruno10 mentioned this pull request Aug 15, 2023

Should return zero records in table if the service API is not enabled #286

Closed

cbruno10 requested changes Aug 15, 2023

View reviewed changes

ParthaI and others added 4 commits October 3, 2023 19:45

Merge branch 'main' of github.com:turbot/steampipe-plugin-gcp into is…

b3c186a

…sue-286

Made changes as per review comments

ca35dd0

Updated the regex pattern

fb92d9c

Updated the regex pattern for better understanding

ab5cc67

cbruno10 requested changes Oct 4, 2023

View reviewed changes

ParthaI and others added 2 commits October 4, 2023 21:29

Updated the doc for ignoring the error by message

944ce9f

Update index.md

f2eac5d

github-actions bot added the stale No recent activity has been detected on this issue/PR and it will be closed label Dec 3, 2023

ParthaI removed the stale No recent activity has been detected on this issue/PR and it will be closed label Dec 4, 2023

github-actions bot added the stale No recent activity has been detected on this issue/PR and it will be closed label Feb 2, 2024

github-actions bot closed this Mar 4, 2024

cbruno10 reopened this Oct 15, 2024

github-actions bot removed the stale No recent activity has been detected on this issue/PR and it will be closed label Oct 15, 2024

pdecat reviewed Nov 28, 2024

View reviewed changes

ParthaI added 3 commits November 28, 2024 15:58

Merge branch 'main' into issue-286

48da7f7

Fixed typo

3a7c37f

Update the config arg

b6a1e4e

pdecat mentioned this pull request Dec 2, 2024

fix: ignore error messages from all error types #688

Merged

fix: ignore error messages from all error types (#688)

b775a5f

* fix: ignore error messages from all error types

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added the default ignore config for service disable error #476

Added the default ignore config for service disable error #476

ParthaI commented Aug 7, 2023

cbruno10 commented Aug 15, 2023

johnsmyth commented Aug 15, 2023

cbruno10 left a comment

cbruno10 Aug 15, 2023

saisirishreddy Aug 15, 2023

multani Sep 1, 2023

ParthaI Oct 4, 2023

cbruno10 Aug 15, 2023

ParthaI Oct 4, 2023

cbruno10 Aug 15, 2023

ParthaI Oct 4, 2023

cbruno10 Aug 15, 2023

ParthaI Oct 4, 2023

cbruno10 left a comment

cbruno10 Oct 4, 2023

ParthaI Oct 4, 2023

cbruno10 Oct 4, 2023

ParthaI Oct 4, 2023 •

edited

Loading

cbruno10 Oct 4, 2023

cbruno10 Oct 4, 2023

ParthaI Oct 4, 2023 •

edited

Loading

github-actions bot commented Dec 3, 2023

github-actions bot commented Feb 2, 2024

github-actions bot commented Mar 4, 2024

pdecat commented Oct 16, 2024

pdecat commented Nov 12, 2024

pdecat Nov 28, 2024 •

edited

Loading

cbruno10 Dec 2, 2024

pdecat Dec 2, 2024

Added the default ignore config for service disable error #476

Are you sure you want to change the base?

Added the default ignore config for service disable error #476

Conversation

ParthaI commented Aug 7, 2023

Integration test logs

Example query results

cbruno10 commented Aug 15, 2023

johnsmyth commented Aug 15, 2023

cbruno10 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cbruno10 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ParthaI Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ParthaI Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Dec 3, 2023

github-actions bot commented Feb 2, 2024

github-actions bot commented Mar 4, 2024

pdecat commented Oct 16, 2024

pdecat commented Nov 12, 2024

pdecat Nov 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ParthaI Oct 4, 2023 •

edited

Loading

ParthaI Oct 4, 2023 •

edited

Loading

pdecat Nov 28, 2024 •

edited

Loading