Skip to content
This repository has been archived by the owner on Oct 3, 2023. It is now read-only.

Add file where function is defined to trace attributes #221

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dz0ny
Copy link

@dz0ny dz0ny commented Jan 17, 2019

a) Not sure how open are you about this.
b) It serves a purpose where the function being trace is same across multiple files and you need to know which file has it.
c) There should probably be some config option to enable this as it might impact performance.

I'm open on suggestions...

@basvanbeek
Copy link
Member

Hi @dz0ny, thanks for the contribution. I think it can be a helpful addition in certain cases. Having said that, there are indeed some concerns:

  • this should not be always on behavior. It will need a PHP configuration switch to allow enabling/disabling this feature (default=disabled).
  • all tests fail at the moment... this functionality needs to be tested and tests currently breaking need to be fixed
  • we have a major update waiting for the PHP extension in OpenCensus Stats implementation #220 so once the design is approved we might ask for a rebase as I expect OpenCensus Stats implementation #220 to take precedence.

@dz0ny
Copy link
Author

dz0ny commented Jan 17, 2019

Yep tests fail because there is new attribute "file" outputted by default.

007+   ["file"]=>

008+   string(43) "/home/circleci/project/ext/tests/common.php"

The problem is that a test will fail because diff (because of the absolute path) will always be depended on the environment. SO the question is how to write tests or test in a way where it does not do exact match but just partial, which is required even if feature is disabled by default.

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

Successfully merging this pull request may close these issues.

None yet

2 participants