Repository navigation
Links to Stackdriver in WebUI #9083
Description
Description
We currently have integration between web UI and Elasticsearch. It was added by this PR: #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
#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 urlBefore 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.
#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.
Activity
mdediana commentedon Jun 1, 2020
@mik-laj I can work on this.
mik-laj commentedon Jun 1, 2020
@mdediana I assigned you to this ticket.
ashb commentedon Jun 15, 2020
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 commentedon Jul 10, 2020
@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 commentedon Jul 10, 2020
@mik-laj I ran into a recursion issue in
StackdriverTaskHandlerwhich I'm trying to figure out. This is how it happens:emitcalls_transportairflow/airflow/providers/google/cloud/log/stackdriver_task_handler.py
Line 140 in 3cc5756
which calls
_clientairflow/airflow/providers/google/cloud/log/stackdriver_task_handler.py
Line 120 in 3cc5756
which calls
get_credentials_and_project_idairflow/airflow/providers/google/cloud/log/stackdriver_task_handler.py
Line 107 in 3cc5756
which calls
log.infoairflow/airflow/providers/google/cloud/utils/credentials_provider.py
Line 211 in 3cc5756
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.infobefore 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:
log.infocall fromget_credentials_and_project_idclienton firstemit, do it eagerly in__init__(i.e., convert thecached_propertyinto an instance variable). This seems to fix the issue but I don't really understand why, as we still end up callinglogging.infoduring the handler initialization, it's just that it's not inemit.Does it all make sense? If so, the second sounds like the right thing to do, what do you think?
mik-laj commentedon Jul 10, 2020
So you want to say that Stackdriver task handler is not working? I am happy to look at it.
mdediana commentedon Jul 11, 2020
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_CREDENTIALSand the recursion problem happened because thelog.infoline was executed. If I switch to usingstackdriver_key_pathinairflow.cfg, the problem stops because that line is not executed. I can move forward with this second option.mik-laj commentedon Jul 11, 2020
@mdediana #9761 Can you look at it?
mdediana commentedon Jul 11, 2020
@mik-laj Looks good, just a couple of tests need fixing.
mdediana commentedon Jul 11, 2020
Backport tests complain about not being able to import
ExternalLoggingMixin. I noticed the mixin was removed fromElasticsearchTaskHandler, which means that links won't show for ES too. I'll try to deleteExternalLoggingMixinand rely on the existence of attributes only (hasattr), seems more stable, but I can't work on it now.mik-laj commentedon Jul 11, 2020
@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 commentedon Jul 11, 2020
@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
ElasticsearchTaskHandlerand remove theExternalLoggingMixindependency for the moment?potiuk commentedon Jul 11, 2020
You need to remove the dependecy @mdediana
mdediana commentedon Jul 11, 2020
@mik-laj @potiuk Done, thanks for the help.