Skip to content

feat: create tests and bump versions#2

Merged
tonyo merged 3 commits intogetsentry:masterfrom
aldy505:feat/bump-version
May 8, 2023
Merged

feat: create tests and bump versions#2
tonyo merged 3 commits intogetsentry:masterfrom
aldy505:feat/bump-version

Conversation

@aldy505
Copy link
Copy Markdown
Contributor

@aldy505 aldy505 commented Apr 25, 2023

I don't know whether this repo is still maintained or accepting PRs whatsoever, but here goes.

Overview:

  • Bump dependencies version. Solves vulnerabilities and compatibility with newer Go versions
  • Minor code changes
    • Changed ioutil.ReadAll to io.ReadAll, the one from ioutil got deprecated
    • Create a buffered channel for the exit signal (SIGINT/SIGTERM) listener
    • Bumped yaml parser, modify the order of the function to strictly parse any yaml.
  • Created tests. Not so thorough because I don't really want to modify the existing code

No golangci-lint and precommit yet, I actually despise those. But if we want to do like on vroom. We'll do that on a separate PR.

If the CI won't run, here's the test results:

PS ~\oss\sentlog> go test -v -cover -covermode=atomic
=== RUN   TestReadConfigFromFile
--- PASS: TestReadConfigFromFile (0.01s)
=== RUN   TestAddDefaultPatterns
--- PASS: TestAddDefaultPatterns (0.00s)
=== RUN   TestReadPatternsFromFile
=== RUN   TestReadPatternsFromFile/Normal_case
time="2023-04-25T11:37:55+07:00" level=info msg="Adding grok patterns from \"~\\AppData\\Local\\Temp\\sentlog-1791219159/normal.txt\""
=== RUN   TestReadPatternsFromFile/Invalid_length
time="2023-04-25T11:37:55+07:00" level=info msg="Adding grok patterns from \"~\\AppData\\Local\\Temp\\sentlog-1791219159/invalid-length.txt\""
=== RUN   TestReadPatternsFromFile/Invalid_pattern
time="2023-04-25T11:37:55+07:00" level=info msg="Adding grok patterns from \"~\\AppData\\Local\\Temp\\sentlog-1791219159/invalid-pattern.txt\""
=== RUN   TestReadPatternsFromFile/Empty_file
time="2023-04-25T11:37:55+07:00" level=info msg="Adding grok patterns from \"~\\AppData\\Local\\Temp\\sentlog-1791219159/empty.txt\""
--- PASS: TestReadPatternsFromFile (0.04s)
    --- PASS: TestReadPatternsFromFile/Normal_case (0.01s)
    --- PASS: TestReadPatternsFromFile/Invalid_length (0.01s)
    --- PASS: TestReadPatternsFromFile/Invalid_pattern (0.02s)
    --- PASS: TestReadPatternsFromFile/Empty_file (0.00s)
=== RUN   TestProcessLine
[Sentry] 2023/04/25 11:37:55 Using release from Git: 69db3b9
[Sentry] 2023/04/25 11:37:55 Sentry client initialized with an empty DSN. Using noopTransport. No events will be delivered.
[Sentry] 2023/04/25 11:37:55 Integration installed: ContextifyFrames
[Sentry] 2023/04/25 11:37:55 Integration installed: Environment
[Sentry] 2023/04/25 11:37:55 Integration installed: Modules
[Sentry] 2023/04/25 11:37:55 Integration installed: IgnoreErrors
[Sentry] 2023/04/25 11:37:55 Event dropped due to noopTransport usage.
time="2023-04-25T11:37:55+07:00" level=info msg="Entry found:"
           auth: -
          bytes: 207
       clientip: 127.0.0.1
    httpversion: 1.1
          ident: -
     rawrequest:
        request: /index.php
       response: 404
      timestamp: 23/Apr/2014:22:58:32 +0200
           verb: GET

--- PASS: TestProcessLine (0.09s)
PASS
        sentlog coverage: 39.5% of statements
ok      sentlog 0.471s

@tonyo
Copy link
Copy Markdown
Contributor

tonyo commented Apr 25, 2023

Hey @aldy505 , thanks for the contributions!
For some background, this repo is basically an experimental hackathon project, so there were no commitments to officially support it going forward, but given the nature/size of the changes -- I'll have a look at the PRs in more detail later this week 👍

In the meantime, could you share your use case(s) for the tool? What kind of services/applications do you use it for, how does it work for you in general, and what kind of features you would like to see added (in case we decide to spend more resources on it)?

@aldy505
Copy link
Copy Markdown
Contributor Author

aldy505 commented Apr 27, 2023

Hi @tonyo sorry for the wait.

I'm using self-hosted Sentry on my company. There are some application for the company's core business module made by third party vendor that usually produces an error to stderr (or maybe stdout, I haven't check it thoroughly). Right now, as the error's not monitored by anything else, we never know that an error happened before a customer filed a complaint and got angry on our customer service department.

My monitoring stack (other than Sentry) includes Grafana's LGTM stack + Alertmanager + grafana-agent to replace Prometheus + Promtail. We can send the logs via promtail to our Grafana Loki instance, but that's about it. There are no error warnings, alerts, whatsoever. That's something we want to explore for a better health (and anger management) at work.

Comment thread .github/workflows/ci.yml Outdated
Copy link
Copy Markdown
Contributor

@tonyo tonyo left a comment

Choose a reason for hiding this comment

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

Looks good, just one note about the test in process_test.go.

Comment thread process_test.go
Comment on lines +18 to +30
processLine(
`127.0.0.1 - - [23/Apr/2014:22:58:32 +0200] "GET /index.php HTTP/1.1" 404 207`,
[]string{"%{COMMONAPACHELOG}"},
g,
sentry.CurrentHub(),
)

processLine(
"",
[]string{"%{COMMONAPACHELOG}"},
g,
sentry.CurrentHub(),
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

At the moment these calls to processLine don't really assert anything (other than not crashing). One potential solution here is to have something similar to TransportMock, and then assert that the proper events were built via Events() method.
Lmk if you want to pursue that, otherwise this is already an improvement.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I went with TransportMock to assert stuff.

@tonyo
Copy link
Copy Markdown
Contributor

tonyo commented Apr 30, 2023

Hi @tonyo sorry for the wait.

I'm using self-hosted Sentry on my company. There are some application for the company's core business module made by third party vendor that usually produces an error to stderr (or maybe stdout, I haven't check it thoroughly). Right now, as the error's not monitored by anything else, we never know that an error happened before a customer filed a complaint and got angry on our customer service department.

My monitoring stack (other than Sentry) includes Grafana's LGTM stack + Alertmanager + grafana-agent to replace Prometheus + Promtail. We can send the logs via promtail to our Grafana Loki instance, but that's about it. There are no error warnings, alerts, whatsoever. That's something we want to explore for a better health (and anger management) at work.

Cool, thanks for sharing 👍 Always interesting to learn what other folks use for observability/monitoring/alerting, and what sort of edge cases are not yet covered by existing tools.

@aldy505 aldy505 requested a review from tonyo May 6, 2023 00:25
@tonyo tonyo merged commit c97330b into getsentry:master May 8, 2023
@tonyo
Copy link
Copy Markdown
Contributor

tonyo commented May 8, 2023

Thanks for the changes 👍

@aldy505 aldy505 deleted the feat/bump-version branch August 11, 2023 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants