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

feat: add loki-logger plugin #9399

Merged
merged 24 commits into from May 29, 2023
Merged

Conversation

bzp2010
Copy link
Contributor

@bzp2010 bzp2010 commented Apr 29, 2023

Description

Create the loki-logger plugin.

Add: #9072

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@bzp2010 bzp2010 self-assigned this Apr 29, 2023
@bzp2010 bzp2010 added enhancement New feature or request plugin labels Apr 30, 2023
@bzp2010 bzp2010 marked this pull request as ready for review April 30, 2023 07:18
@bzp2010 bzp2010 linked an issue Apr 30, 2023 that may be closed by this pull request
docs/en/latest/plugins/loki-logger.md Outdated Show resolved Hide resolved
Gallardot
Gallardot previously approved these changes May 6, 2023
apisix/plugins/loki-logger.lua Show resolved Hide resolved


=== TEST 4: check loki log
--- config
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can write a public function to query the log

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@soulbird please help to check

@bzp2010 bzp2010 requested review from soulbird and Gallardot May 8, 2023 02:39
@Revolyssup
Copy link
Contributor

Revolyssup commented May 23, 2023

@bzp2010 Instead of fetching the loki logs in our function in the tests, what if we use https://vector.dev/docs/reference/configuration/sinks/loki/ ? This is being used in tcp-logger, and there are other pull requests to add the same for other loggers as well. What do you think?

@monkeyDluffy6017
Copy link
Contributor

@Sn0rt please help to review

@monkeyDluffy6017
Copy link
Contributor

LGTM, could you provide the chinese doc?

@monkeyDluffy6017 monkeyDluffy6017 added wait for update wait for the author's response in this issue/PR and removed enhancement New feature or request plugin labels May 23, 2023
@bzp2010
Copy link
Contributor Author

bzp2010 commented May 26, 2023

Hi, @monkeyDluffy6017

Since I'm busy with some other things, I can't afford to finish the Chinese document for now. But I think if the code and test of this PR are fine, we can merge it first, and once it is merged, I will create an issue and mark it as the good first issue, expecting some potential contributor to help finish it.

@bzp2010
Copy link
Contributor Author

bzp2010 commented May 26, 2023

Hi, @Revolyssup.

I'd like to understand where this essentially lies in terms of consideration.

I don't know much about vectors, but I think it makes sense to track upstream (i.e. Grafana Loki itself). If we use vector to receive logs, we may have to wait for it once the protocol is upgraded.

@monkeyDluffy6017 monkeyDluffy6017 added approved and removed wait for update wait for the author's response in this issue/PR labels May 26, 2023
@monkeyDluffy6017
Copy link
Contributor

@Revolyssup please help to check

@monkeyDluffy6017
Copy link
Contributor

Since I'm busy with some other things, I can't afford to finish the Chinese document for now. But I think if the code and test of this PR are fine, we can merge it first, and once it is merged, I will create an issue and mark it as the good first issue, expecting some potential contributor to help finish it.

Please remember to create a issue

@monkeyDluffy6017 monkeyDluffy6017 merged commit 66cd80f into apache:master May 29, 2023
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: loki-logger plugin
6 participants