Repository: dbt-labs/dbt-core
Pull request: Fix/bigquery job label length
resolves #3612 

<!---
  Include the number of the issue addressed by this PR above if applicable.
  PRs for code changes without an associated issue *will not be merged*.
  See CONTRIBUTING.md for more information.

  Example:
    resolves #1234
-->


### Description

<!--- Describe the Pull Request here -->
![image](https://user-images.githubusercontent.com/19176976/128535366-5e24a25b-a4fb-4af4-8034-4cc3edcbbd65.png)
![image](https://user-images.githubusercontent.com/19176976/128530375-1de3ea67-582c-4648-bfd1-8532da819a52.png)
> Screenshots based on using `jaffle_shop` dbt project along with using an [example query comment macro](https://docs.getdbt.com/reference/project-configs/query-comment/#advanced-use-a-macro-to-generate-a-comment)
- Updated the existing `_sanitize_label` function to validate a global length variable: `_VALIDATE_LABEL_LENGTH_LIMIT`
- Added a passing test with 3 scenarios with randomly generated strings each time you run the test
- If an exception is raised, it provides clearer logging in the terminal for the error reason being the length limit being exceeded along with the currently sanitized label
- I will update the `CHANGELOG.md` after the core logic is approved


### Checklist
 - [x] I have signed the [CLA](https://docs.getdbt.com/docs/contributor-license-agreements)
 - [x] I have run this code in development and it appears to resolve the stated issue
 - [x] This PR includes tests, or tests are not required/relevant for this PR
 - [x] I have updated the `CHANGELOG.md` and added information about my change to the "dbt next" section.
 


Problem statement:
Raise a more helpful error if BigQuery job label is too long
### Describe the bug
After migrating to v0.20.0 my jobs keep failing with the following error:

```
Database Error in model stg_facebookads__ads (models/staging/facebook_ads/stg_facebookads__ads.sql)                                                                                                         
  Label value "__database____REDACTED____schema____dev_edbizarro____identifier____stg_facebookads__ads__" has invalid characters.                                                                         
  compiled SQL at target/run/insights_lab/models/staging/facebook_ads/stg_facebookads__ads.sql
```

### Steps To Reproduce
Config labels in dbt_project.yml
```yml
query-comment:
  comment: "{{ query_comment(node) }}"
  append: false
  job-label: true
``` 

Run any model

### Expected behavior

### Screenshots and log output
If applicable, add screenshots or log output to help explain your problem.

### System information
**Which database are you using dbt with?**
- [ ] postgres
- [ ] redshift
- [x] bigquery
- [ ] snowflake
- [ ] other (specify: ____________)


**The output of `dbt --version`:**
```
 ❯ dkc exec dbt dbt --version                                                                                                                                                                    [17:52:32]
installed version: 0.20.0
   latest version: 0.20.0

Up to date!

Plugins:
  - redshift: 0.20.0
  - snowflake: 0.20.0
  - bigquery: 0.20.0
  - postgres: 0.20.0

```

**The operating system you're using:**
Arch Linux

**The output of `python --version`:**

Running through official docker image

### Additional context
Add any other context about the problem here.

Hints from issue discussion:
@edbizarro I see you switched on `query-comment.job-label`, which is new in v0.20.0. The way that feature works:
- Try reading your query comment as a dictionary. If it can, pass each key-value pair as a separate job label. "Sanitize" each value by replacing whitespace and special characters with underscores.
- If your query comment can't be read as a dictionary, pass the entire (sanitized) query comment as a single job label.

The latter is what's happening here. Unfortunately, the error BigQuery is giving back is a bit misleading:
```
Label value "__database____REDACTED____schema____dev_edbizarro____identifier____stg_facebookads__ads__" has invalid characters.
```
The real issue here is that labels are limited to 63 characters in length ([docs](https://cloud.google.com/blog/products/gcp/labelling-and-grouping-your-google-cloud-platform-resources)), and this string is 89 characters in length. If I shorten the string to 63 characters, everything works just fine.

In the original PR for this feature, we discussed potential approaches for handling too-long labels: https://github.com/dbt-labs/dbt/pull/3145#discussion_r598050639. The options are:
1. Truncate, hash, or otherwise handle the label length within dbt. This would happen silently, and could result in indistinguishable label values.
2. Raise an error within dbt.
3. Do nothing, and return any errors from BigQuery.

We picked the third option. Given the lack of clarity BigQuery's error message, and the ensuing confusion indicated by this issue, I think there's good reason to prefer the second: I think we should raise a compilation error any time `query-comment.job-label` is switched on and a label value would be >63 characters.

That should be a straightforward change. Is it something you'd be interested in contributing @edbizarro?

In any event, you'll need to work around this error by:
1. Refactoring your `query_comment` macro to return a dictionary
2. Refactoring your `query_comment` macro to return a shorter string (post-sanitization)
3. Switching off `query-comment.job-label`
@jtcohen6 I'm happy to take this issue on if @edbizarro doesn't want to!
@sungchun12 I'd love that!
@jtcohen6 This is officially in my personal backlog that I'll spend the next week or so focusing on!

I'll be diving into #3145. And I'm assuming the solution to this problem will be extending the validations and raising specific error messages in functions like: [this](https://github.com/jmcarp/dbt/blob/044a6c6ea4966f04a7f27bbd506ad172d01a6149/plugins/bigquery/dbt/adapters/bigquery/connections.py#L597) and/or create another function dedicated to verifying string length after sanitization.
@sungchun12 Sure! I really want to take this challenge but unfortunately I'm little short in time this next weeks so i would love that someone taking on this, thanks!
@sungchun12 Yes! I think `_sanitize_label` raising a `ValidationException` if passed a string longer than 63 characters will get the job done.

Commit message: Merge branch 'develop' of https://github.com/dbt-labs/dbt into fix/bigquery-job-label-length

Patch to review:
diff --git a/plugins/bigquery/dbt/adapters/bigquery/connections.py b/plugins/bigquery/dbt/adapters/bigquery/connections.py
index 137b02d4f48..bf21b1bafd0 100644
--- a/plugins/bigquery/dbt/adapters/bigquery/connections.py
+++ b/plugins/bigquery/dbt/adapters/bigquery/connections.py
@@ -595,9 +595,18 @@ def _is_retryable(error):
 
 _SANITIZE_LABEL_PATTERN = re.compile(r"[^a-z0-9_-]")
 
+_VALIDATE_LABEL_LENGTH_LIMIT = 63
+
 
 def _sanitize_label(value: str) -> str:
     """Return a legal value for a BigQuery label."""
     value = value.strip().lower()
     value = _SANITIZE_LABEL_PATTERN.sub("_", value)
-    return value
+    value_length = len(value)
+    if value_length > _VALIDATE_LABEL_LENGTH_LIMIT:
+        error_msg = (
+            f"Current label length {value_length} is greater than length limit: {_VALIDATE_LABEL_LENGTH_LIMIT} | Current sanitized label: {value}"
+        )
+        raise Exception(error_msg)
+    else:
+        return value
diff --git a/test/unit/test_bigquery_adapter.py b/test/unit/test_bigquery_adapter.py
index 5f7d79054e6..cc9de006199 100644
--- a/test/unit/test_bigquery_adapter.py
+++ b/test/unit/test_bigquery_adapter.py
@@ -1,6 +1,8 @@
 import agate
 import decimal
 import json
+import string
+import random
 import re
 import pytest
 import unittest
@@ -17,7 +19,7 @@
 from dbt.adapters.bigquery import BigQueryRelation
 from dbt.adapters.bigquery import Plugin as BigQueryPlugin
 from dbt.adapters.bigquery.connections import BigQueryConnectionManager
-from dbt.adapters.bigquery.connections import _sanitize_label
+from dbt.adapters.bigquery.connections import _sanitize_label, _VALIDATE_LABEL_LENGTH_LIMIT
 from dbt.adapters.base.query_headers import MacroQueryStringSetter
 from dbt.clients import agate_helper
 import dbt.exceptions
@@ -972,3 +974,18 @@ def test_convert_time_type(self):
 )
 def test_sanitize_label(input, output):
     assert _sanitize_label(input) == output
+
+
+@pytest.mark.parametrize(
+    "label_length",
+    [64, 65, 100],
+)
+def test_sanitize_label_length(label_length):
+    random_string = "".join(
+        random.choice(string.ascii_uppercase + string.digits)
+        for i in range(label_length)
+    )
+    test_error_msg = f"Current label length {label_length} is greater than length limit: {_VALIDATE_LABEL_LENGTH_LIMIT} | Current sanitized label: {random_string.lower()}"
+    with pytest.raises(Exception) as error_info:
+        _sanitize_label(random_string)
+    assert error_info.value.args[0] == test_error_msg


Code review comments:
diff_hunk: @@ -595,9 +595,18 @@ def _is_retryable(error):
 
 _SANITIZE_LABEL_PATTERN = re.compile(r"[^a-z0-9_-]")
 
+_VALIDATE_LABEL_LENGTH_LIMIT = 63
+
 
 def _sanitize_label(value: str) -> str:
     """Return a legal value for a BigQuery label."""
     value = value.strip().lower()
     value = _SANITIZE_LABEL_PATTERN.sub("_", value)
-    return value
+    value_length = len(value)
+    if value_length > _VALIDATE_LABEL_LENGTH_LIMIT:
+        error_msg = (
+            f"Current label length {value_length} is greater than length limit: {_VALIDATE_LABEL_LENGTH_LIMIT} | Current sanitized label: {value}"
+        )
+        raise Exception(error_msg)
path: plugins/bigquery/dbt/adapters/bigquery/connections.py
text: @user1:
```suggestion
        raise RuntimeException(error_msg)
```
This way, instead of seeing `Unhandled error while executing model...`, you'll just see `ERROR` and the runtime error reported in the error summary.

diff_hunk: @@ -595,9 +595,18 @@ def _is_retryable(error):
 
 _SANITIZE_LABEL_PATTERN = re.compile(r"[^a-z0-9_-]")
 
+_VALIDATE_LABEL_LENGTH_LIMIT = 63
+
 
 def _sanitize_label(value: str) -> str:
     """Return a legal value for a BigQuery label."""
     value = value.strip().lower()
     value = _SANITIZE_LABEL_PATTERN.sub("_", value)
-    return value
+    value_length = len(value)
+    if value_length > _VALIDATE_LABEL_LENGTH_LIMIT:
+        error_msg = (
+            f"Current label length {value_length} is greater than length limit: {_VALIDATE_LABEL_LENGTH_LIMIT} | Current sanitized label: {value}"
path: plugins/bigquery/dbt/adapters/bigquery/connections.py
text: @user1:
Our style formatter (flake8) says this line is too long, which is why one of the CI tests is failing:
```
plugins/bigquery/dbt/adapters/bigquery/connections.py:608:100: E501 line too long (146 > 99 characters)
```
It's also a bit long on the CLI. How about making this two lines?
```suggestion
            f"Job label length {value_length} is greater than length limit: " \
            f"{_VALIDATE_LABEL_LENGTH_LIMIT}\n" \
            f"Current sanitized label: {value}"
```