Airflow: Links to Stackdriver in WebUI

[mik-laj]

Description

We currently have integration between web UI and Elasticsearch. It was added by this PR: https://github.com/apache/airflow/pull/5164

This allows easier access to the Elasticsearch interface. We recently added a task handler that allows you to save logs to the Stackdriver service
https://github.com/apache/airflow/pull/6660
I would be happy if a similar button also appears when StackdriverTaskHandler is used. Stackdriver has a much more friendly interface than Airflow, so access to it would be very helpful. It is also a fully managed service, so anyone can use it very easily.

To generate the Stackdriver URL, you can use the following code:

    LOG_VIEWER_BASE_URL = "https://console.cloud.google.com/logs/viewer"

    @property
    def _resource_path(self):
        segments = [self.resource.type]

        for key, value in self.resource.labels:
            segments += [key]
            segments += [value]

        return "/".join(segments)

    def get_external_log_url(self, task_instance: TaskInstance, try_number: int) -> str:
        """
        Creates an address for an external log collecting service.
        :param task_instance: task instance object
        :type: task_instance: TaskInstance
        :param try_number: task instance try_number to read logs from.
        :type try_number: Optional[int]
        :return: URL to the external log collection service
        :rtype: str
        """
        project_id = self._client.project

        ti_labels = self._task_instance_to_labels(task_instance)
        ti_labels[self.LABEL_TRY_NUMBER] = str(try_number)

        log_filter = self._prepare_log_filter(ti_labels)

        url_query_string = {
            'project': project_id,
            'interval': 'NO_LIMIT',
            'resource': self._resource_path,
            'advancedFilter': log_filter,
        }

        url = f"{self.LOG_VIEWER_BASE_URL}?{urllib.parse.urlencode(url_query_string)}"
        return url

Before making this change, I think it's worth adding the missing tests PR, which added external links for Elasticsearch does not introduce any new tests, so it is advisable to supplement them before starting work.
You can use capture_templates method to do it.
https://github.com/apache/airflow/pull/8505

If anyone is interested in this task, I am willing to provide all the necessary tips and information.

If you haven't used the GCP yet, after creating the account you will get $300, which will allow you to test change and get to know this service better.

[mdediana]

@mik-laj I can work on this.

[mik-laj]

@mdediana I assigned you to this ticket.

[ashb]

We should ideally do this such that any of the task logging handlers could provide custom log links -- i.e. don't just hard code support for ES or StackDriver in to the UI.

[mik-laj]

@mdediana Hi. I am happy that we have already done the first PR. My users are asking about this feature. Do you need help now?

[mdediana]

@mik-laj I ran into a recursion issue in StackdriverTaskHandler which I'm trying to figure out. This is how it happens:

emit calls _transport
https://github.com/apache/airflow/blob/3cc5756d04c7d3fd14b71e9ec6f5f906d5a24212/airflow/providers/google/cloud/log/stackdriver_task_handler.py#L140

which calls _client
https://github.com/apache/airflow/blob/3cc5756d04c7d3fd14b71e9ec6f5f906d5a24212/airflow/providers/google/cloud/log/stackdriver_task_handler.py#L120

which calls get_credentials_and_project_id
https://github.com/apache/airflow/blob/3cc5756d04c7d3fd14b71e9ec6f5f906d5a24212/airflow/providers/google/cloud/log/stackdriver_task_handler.py#L107

which calls log.info
https://github.com/apache/airflow/blob/3cc5756d04c7d3fd14b71e9ec6f5f906d5a24212/airflow/providers/google/cloud/utils/credentials_provider.py#L211

which apparently starts it all over again and then it breaks because maximum recursion depth is reached.

If I understand this correctly, the problem is that we are calling logging.info before initialization has finished, so a new initialization starts and so on.

I tried two things which prevent the error from happening, my plan is to investigate it further tomorrow and fix it. The two things:

• Removing the log.info call from get_credentials_and_project_id
• Instead of lazily initializing client on first emit, do it eagerly in __init__ (i.e., convert the cached_property into an instance variable). This seems to fix the issue but I don't really understand why, as we still end up calling logging.info during the handler initialization, it's just that it's not in emit.

Does it all make sense? If so, the second sounds like the right thing to do, what do you think?

[mik-laj]

So you want to say that Stackdriver task handler is not working? I am happy to look at it.

[mdediana]

Thanks for the help.

I found something, it still needs fixing but it's not a blocker for this issue.

I was using export GOOGLE_APPLICATION_CREDENTIALS and the recursion problem happened because the log.info line was executed. If I switch to using stackdriver_key_path in airflow.cfg, the problem stops because that line is not executed. I can move forward with this second option.

[mik-laj]

@mdediana https://github.com/apache/airflow/pull/9761 Can you look at it?

[mdediana]

@mik-laj Looks good, just a couple of tests need fixing.

[mdediana]

Backport tests complain about not being able to import ExternalLoggingMixin. I noticed the mixin was removed from ElasticsearchTaskHandler, which means that links won't show for ES too. I'll try to delete ExternalLoggingMixin and rely on the existence of attributes only (hasattr), seems more stable, but I can't work on it now.

[mik-laj]

@mdediana This is expected. This class cannot be used in backport packages unless this class is available for 1.10. When we release a new Airflow with this class, we can also release a backport package that depends on this class. Otherwise, we will have a class that no one can use. In Airflow 2.0 we don't need a backport package, because this code is installed together with Airflow.

[mdediana]

@mik-laj Not sure if I follow, so it is OK to have the backport tests failing in this case? Or should I do the same as was done with ElasticsearchTaskHandler and remove the ExternalLoggingMixin dependency for the moment?

[potiuk]

You need to remove the dependecy @mdediana

[mdediana]

@mik-laj @potiuk Done, thanks for the help.