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

fix: include frontend with oss query-service #7247

Merged
merged 2 commits into from
Mar 13, 2025

Conversation

davispuh
Copy link
Contributor

@davispuh davispuh commented Mar 8, 2025

Summary

#6696 updated ee/query-service/Dockerfile but didn't update pkg/query-service/Dockerfile

This causes query-service fail to start when using oss tagged images.
Workaround is to pass SIGNOZ_WEB_ENABLED=false env but it's not obvious because previously it just worked.

{
  "level": "FATAL",
  "timestamp": "2025-03-08T21:28:46.757Z",
  "caller": "query-service/main.go:99",
  "msg": "Failed to create signoz struct",
  "error": "cannot access web directory: stat /etc/signoz/web: no such file or directory",
  "stacktrace": "main.main\n\t/home/runner/work/signoz/signoz/pkg/query-service/main.go:99\nruntime.main\n\t/home/runner/go/pkg/mod/golang.org/[email protected]/src/runtime/proc.go:271"
}

Related Issues / PR's

Screenshots

NA

Affected Areas and Manually Tested Areas


Important

Fixes query-service startup failure by including frontend directory in pkg/query-service/Dockerfile.

  • Dockerfile Update:
    • In pkg/query-service/Dockerfile, added COPY frontend/build/ /etc/signoz/web/ to include the frontend directory in the oss tagged images.
  • Behavior:
    • Fixes startup failure of query-service due to missing frontend directory when using oss tagged images.
    • Previously required workaround of setting SIGNOZ_WEB_ENABLED=false is no longer necessary.

This description was created by Ellipsis for bcd62bc. It will automatically update as commits are pushed.

Copy link

welcome bot commented Mar 8, 2025

Welcome to the SigNoz community! Thank you for your first pull request and making this project better. 🤗

@CLAassistant
Copy link

CLAassistant commented Mar 8, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@ellipsis-dev ellipsis-dev bot 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 to me! Reviewed everything up to bcd62bc in 1 minute and 0 seconds

More details
  • Looked at 14 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 3 drafted comments based on config settings.
1. pkg/query-service/Dockerfile:27
  • Draft comment:
    Ensure the target directory exists before copying. Docker often creates it, but explicitly creating the directory may improve clarity.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50%
  1. Docker's COPY command automatically creates target directories if they don't exist. 2. Creating directories explicitly is not a common practice in Dockerfiles unless there's a specific permission/ownership requirement. 3. The comment starts with "Ensure that..." which violates our rules. 4. The comment doesn't point out an actual problem - it's just a style suggestion that doesn't improve functionality.
    The comment might have merit if there were specific permission requirements for the web directory that needed to be set before copying.
    Even if directory permissions were a concern, they could be set after the COPY command. The current approach is standard Docker practice.
    Delete the comment as it starts with "Ensure that...", doesn't identify a real problem, and suggests an unnecessary change to standard Docker practices.
2. pkg/query-service/Dockerfile:26
3. pkg/query-service/Dockerfile:27
  • Draft comment:
    Consider adding a permission adjustment (chmod/chown) for /etc/signoz/web/ if query-service needs non-root access.
  • Reason this comment was not posted:
    Confidence changes required: 33% <= threshold 50%
    None

Workflow ID: wflow_TddtpkEENbEBXcjD


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@grandwizard28
Copy link
Collaborator

Hi @davispuh,
Thank you for the PR!

Apart from the changes mentioned here, we also need to change the corresponding github workflows for this to happen. We are in the process of revamping our CI and we plan to undertake this there itself.

@grandwizard28 grandwizard28 changed the title Include frontend with oss query-service fix: include frontend with oss query-service Mar 13, 2025
@primus-bot primus-bot bot removed the safe-to-test label Mar 13, 2025
@grandwizard28 grandwizard28 merged commit 50c1af2 into SigNoz:main Mar 13, 2025
26 of 29 checks passed
Copy link

welcome bot commented Mar 13, 2025

Congrats on merging your first pull request!
minion-party
We here at SigNoz are proud of you! 🥳

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants