Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Core feature] Dynamic log links #4773

Open
2 tasks done
eapolinario opened this issue Jan 24, 2024 · 2 comments · Fixed by #4774
Open
2 tasks done

[Core feature] Dynamic log links #4773

eapolinario opened this issue Jan 24, 2024 · 2 comments · Fixed by #4774
Assignees
Labels
backlogged For internal use. Reserved for contributor team workflow. enhancement New feature or request

Comments

@eapolinario
Copy link
Contributor

Motivation: Why do you think this is important?

We need a way to indicate from a task that additional log links are required.

This is useful, for example, in the case of flyin, where users can specify the port used in the VSCode server spawn in the task. Allowing for the task to emit a log link that exposes the VScode server on a specific port.

Other plugins and extensions can make use of this feature as well, like extensions that can benefit from other information present in the task template to generate log links.

Goal: What should the final outcome look like, ideally?

We should be able to configure dynamic log links in the logs configuration, like so:

plugins:
  logs:
    dynamic-log-links-enabled: true
    dynamic-log-links:
      flyin: http://flyin.vs:{{ .taskConfig.port }}/{{ .podName }}

Notice the way the the configuration passed from the task is accessed, in this case assume that the task is setting port in its task template, so in order to access that in the template we use the prefix .taskConfig in the template.

The dynamic-log-links is a map of string to TemplateLogPlugin, where the key is the name of the dynamic log link, which is used to identify which logs should be applied in the case of a specific task. For example, consider the case of 2 dynamic logs links being enabled in the configuration, i.e.:

plugins:
  logs:
    dynamic-log-links-enabled: true
    dynamic-log-links:
      abc: http://foo.bar/{{ .taskConfig.V1 }}
      def: http://baz/{{ .taskConfig.V2 }}

and the task only specifies def in its list of dynamic log links:

@task(dynamic_log_link="def")
def t():
  ...

(I'm abusing the notation a little bit, since dynamic_log_link is not a field of the task decorator at the moment) Assume that by setting dynamic_log_link="def" I'm setting a field in t's task template to indicate that I want def to be generated as a dynamic log link. In the backend, we use that information to ensure that only def is applied to executions of t.

The idea is that at registration time tasks are going to use a comma-separated property called link_type set in the task template metadata and we're going to use that to control which dynamic log links are going to produced for that task.

Describe alternatives you've considered

I implemented a version of this idea in #4610, but that failed short when it came to extensibility.

Propose: Link/Inline OR Additional context

No response

Are you sure this issue hasn't been raised already?

  • Yes

Have you read the Code of Conduct?

  • Yes
@eapolinario eapolinario added enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers labels Jan 24, 2024
@eapolinario eapolinario self-assigned this Jan 24, 2024
Copy link

dosubot bot commented Jan 24, 2024

🤖

It's great to see your detailed proposal for implementing dynamic log links. Your motivation and goal are well articulated, and it's clear that you've put a lot of thought into this feature. Encouraging users to contribute to the repository by creating a PR with your proposed solution would be a fantastic next step. Keep up the good work!


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@eapolinario eapolinario mentioned this issue Jan 24, 2024
3 tasks
@eapolinario eapolinario removed the untriaged This issues has not yet been looked at by the Maintainers label Jan 25, 2024
@eapolinario eapolinario reopened this Mar 15, 2024
@eapolinario
Copy link
Contributor Author

We're going to support a more generic version of dynamic log links. In this extension, we are going to get the log link template from the task template itself.

@eapolinario eapolinario added the backlogged For internal use. Reserved for contributor team workflow. label Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlogged For internal use. Reserved for contributor team workflow. enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant